From 020fbb891e1195b0efbcdf1aa06261c1a434641e Mon Sep 17 00:00:00 2001 From: Gabriel Kaszewski Date: Mon, 12 Oct 2026 19:29:54 +0200 Subject: [PATCH 1/4] =?UTF-8?q?feat:=20always-on=20Insights=20=E2=80=94=20?= =?UTF-8?q?correlation=20analysis=20over=20viewing=20habits?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Conservative, live insights (ADR-0010) derived from each user's diary: rating lift by genre (first-time watches only), genre seasonality vs a uniform 1/12 month baseline, medium weekend-lift, and watch-time rhythm. - domain: WatchRow model, InsightsQuery port, pure insights_analyzer with all honesty thresholds (sample minimums, strength cutoffs, suppressions for import noise), unit-tested - adapters: thin Sqlite/Postgres fetch twins, reusing WrapUp's genre fetch - application: get_insights use case, InsightsGroup in Deps - composition/server: wiring through Repositories/DatabaseOutput - presentation: GET /api/v1/insights handler, typed serde-tagged DTOs, openapi doc - spa: /insights route (shadcn cards), insights feature hook, profile entry link, en locale strings - server test: end-to-end seed sqlite → HTTP → rating lift + rhythm Also records ADR-0010, defines "Insight" in the glossary, and includes pre-existing worktree comment-stripping cleanups. --- CONTEXT.md | 4 + Makefile | 113 +----- .../src/federation_event_bridge.rs | 1 - .../activitypub/src/federation_ports.rs | 13 - crates/adapters/activitypub/src/lib.rs | 7 - crates/adapters/activitypub/src/urls.rs | 3 - crates/adapters/adapter-common/src/lib.rs | 27 -- .../adapters/image-converter/src/backfill.rs | 2 +- .../image-converter/src/tests/handler.rs | 2 - crates/adapters/importer/src/parsers/xlsx.rs | 2 - crates/adapters/importer/src/tests/mapper.rs | 9 +- crates/adapters/metadata/src/tmdb.rs | 3 +- crates/adapters/nats/src/consumer.rs | 1 - crates/adapters/object-storage/src/lib.rs | 1 - crates/adapters/plex/src/lib.rs | 3 - crates/adapters/poster-sync/src/lib.rs | 1 - .../adapters/postgres-event-queue/src/lib.rs | 2 - crates/adapters/postgres-social/src/lib.rs | 8 - crates/adapters/postgres/src/diary.rs | 5 - crates/adapters/postgres/src/insights.rs | 81 ++++ crates/adapters/postgres/src/lib.rs | 4 + crates/adapters/postgres/src/movie_dedup.rs | 6 - crates/adapters/postgres/src/wrapup.rs | 3 +- crates/adapters/sqlite-social/src/lib.rs | 8 - crates/adapters/sqlite/src/insights.rs | 80 ++++ crates/adapters/sqlite/src/lib.rs | 4 + crates/adapters/sqlite/src/models.rs | 2 - crates/adapters/sqlite/src/movie_dedup.rs | 6 - crates/adapters/sqlite/src/tests/diary.rs | 17 +- crates/adapters/sqlite/src/wrapup.rs | 3 +- crates/api-types/src/diary.rs | 1 - crates/api-types/src/import.rs | 7 - crates/api-types/src/insights.rs | 49 +++ crates/api-types/src/lib.rs | 2 + crates/api-types/src/movies.rs | 3 - crates/api-types/src/search.rs | 8 - crates/api-types/src/social.rs | 4 +- crates/api-types/src/users.rs | 1 - crates/application/src/deps.rs | 6 + crates/application/src/diary/get_user_feed.rs | 3 - .../src/diary/tests/review_logger.rs | 2 - .../application/src/import/apply_profile.rs | 2 - crates/application/src/import/deps.rs | 5 - .../src/import/get_mapping_stage.rs | 8 - .../src/import/get_preview_stage.rs | 16 - .../src/import/tests/apply_mapping.rs | 2 - crates/application/src/insights/deps.rs | 7 + crates/application/src/insights/get.rs | 17 + crates/application/src/insights/mod.rs | 2 + crates/application/src/insights/tests/get.rs | 93 +++++ crates/application/src/lib.rs | 1 + .../src/movies/discovery_indexer.rs | 3 - crates/application/src/movies/enrich_movie.rs | 8 - crates/application/src/movies/resolve.rs | 3 - crates/application/src/services.rs | 3 - .../application/src/social/local_service.rs | 7 - crates/application/src/tests/services.rs | 4 - .../application/src/users/authorize_admin.rs | 9 - crates/application/src/users/deps.rs | 12 - .../src/users/get_federated_profile.rs | 12 - .../src/users/get_local_profile.rs | 4 - .../src/users/get_profile_settings.rs | 13 - .../tests/get_federated_profile_stats.rs | 8 - .../src/users/tests/get_local_profile.rs | 8 - .../src/users/tests/get_page_viewer.rs | 3 - .../src/users/tests/get_profile_settings.rs | 4 - .../src/users/tests/resolve_username_to_id.rs | 3 - .../src/watchlist/get_watchlist_for_owner.rs | 16 - .../tests/get_watchlist_for_owner.rs | 12 - .../src/wrapup/get_ready_report.rs | 17 - .../src/wrapup/tests/get_ready_report.rs | 4 - crates/composition/src/build.rs | 11 +- crates/composition/src/factory.rs | 3 + crates/composition/src/repositories.rs | 13 +- crates/composition/src/tests/build.rs | 16 +- crates/domain/src/models/federation.rs | 7 - crates/domain/src/models/insights.rs | 66 ++++ crates/domain/src/models/mod.rs | 2 + crates/domain/src/models/person.rs | 1 - crates/domain/src/models/review.rs | 2 - crates/domain/src/models/search.rs | 5 - crates/domain/src/ports/events.rs | 3 - crates/domain/src/ports/federation.rs | 6 - crates/domain/src/ports/images.rs | 1 - crates/domain/src/ports/insights.rs | 12 + crates/domain/src/ports/media_server.rs | 2 - crates/domain/src/ports/mod.rs | 2 + crates/domain/src/ports/movie.rs | 9 - crates/domain/src/ports/noop.rs | 5 - crates/domain/src/ports/person.rs | 8 - crates/domain/src/ports/search.rs | 4 - crates/domain/src/ports/social.rs | 35 -- crates/domain/src/ports/watchlist.rs | 3 - .../domain/src/services/insights_analyzer.rs | 343 ++++++++++++++++ crates/domain/src/services/mod.rs | 1 + .../src/services/tests/insights_analyzer.rs | 365 ++++++++++++++++++ crates/domain/src/testing/fakes.rs | 3 - crates/domain/src/testing/in_memory.rs | 7 - crates/domain/src/testing/insights.rs | 52 +++ crates/domain/src/testing/mod.rs | 2 + crates/domain/src/value_objects/ids.rs | 5 - crates/domain/src/value_objects/instance.rs | 5 - crates/domain/src/value_objects/social.rs | 2 - crates/domain/src/value_objects/user.rs | 2 - crates/presentation/src/csrf.rs | 1 - crates/presentation/src/handlers/insights.rs | 81 ++++ crates/presentation/src/handlers/mod.rs | 1 + crates/presentation/src/handlers/social.rs | 3 - crates/presentation/src/openapi/insights.rs | 8 + crates/presentation/src/openapi/mod.rs | 2 + crates/presentation/src/routes.rs | 1 + crates/presentation/src/tests/api_handlers.rs | 15 - crates/presentation/src/tests/context.rs | 9 - crates/presentation/src/tests/extractors.rs | 16 +- crates/server/src/main.rs | 1 + crates/server/tests/api_test.rs | 155 +++++++- crates/tui/src/app.rs | 2 - crates/tui/src/main.rs | 6 - crates/tui/src/tests/app.rs | 3 - docs/adr/0010-conservative-live-insights.md | 62 +++ spa/bun.lock | 4 + spa/src/features/insights.ts | 61 +++ spa/src/locales/en.json | 19 + spa/src/routeTree.gen.ts | 21 + spa/src/routes/_app/insights.tsx | 263 +++++++++++++ spa/src/routes/_app/profile.tsx | 11 +- 126 files changed, 1922 insertions(+), 639 deletions(-) create mode 100644 crates/adapters/postgres/src/insights.rs create mode 100644 crates/adapters/sqlite/src/insights.rs create mode 100644 crates/api-types/src/insights.rs create mode 100644 crates/application/src/insights/deps.rs create mode 100644 crates/application/src/insights/get.rs create mode 100644 crates/application/src/insights/mod.rs create mode 100644 crates/application/src/insights/tests/get.rs create mode 100644 crates/domain/src/models/insights.rs create mode 100644 crates/domain/src/ports/insights.rs create mode 100644 crates/domain/src/services/insights_analyzer.rs create mode 100644 crates/domain/src/services/tests/insights_analyzer.rs create mode 100644 crates/domain/src/testing/insights.rs create mode 100644 crates/presentation/src/handlers/insights.rs create mode 100644 crates/presentation/src/openapi/insights.rs create mode 100644 docs/adr/0010-conservative-live-insights.md create mode 100644 spa/src/features/insights.ts create mode 100644 spa/src/routes/_app/insights.tsx diff --git a/CONTEXT.md b/CONTEXT.md index 3df02da2..3e99773d 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -36,6 +36,10 @@ _Avoid_: Challenge, resolution, target A generated summary report of viewing activity over a date range — statistics, trends, highlights, top directors/actors/genres. Can be personal (one user) or global (all users). Generated asynchronously. Shown to users as "Year in Review." _Avoid_: Stats page, recap, summary +**Insight**: +A statement about a user's viewing behavior derived from their Reviews and linked Movie metadata, produced by correlation analysis. Always a comparison — relative to the user's own baseline (their average rating, their genre frequencies, their typical hours) — never a plain ranking or count. "You rate thrillers higher than your average" is an Insight; "thriller is your top genre" is a statistic. +_Avoid_: Correlation (implementation), fun fact, finding + **User**: A registered account with a username, email, and profile (display name, bio, avatar, banner). Can be Standard or Admin. _Avoid_: Account, member, profile (as a synonym for the whole User) diff --git a/Makefile b/Makefile index 7069a4fd..95d8cce6 100644 --- a/Makefile +++ b/Makefile @@ -1,10 +1,8 @@ .DEFAULT_GOAL := check -# Run the full local check suite — same order as CI would. check: fmt-check clippy test check-appcontext check-handler-deps check-handler-repos check-presentation-adapters check-federation-off-social check-social-crates-are-ap-free @echo "✅ All checks passed" -# Enforce that no application use case imports AppContext (god-object guard). check-appcontext: @if grep -rn "AppContext" crates/application/src --include="*.rs" | grep -q .; then \ echo "❌ AppContext found in application crate:"; \ @@ -13,20 +11,6 @@ check-appcontext: fi @echo "✅ No AppContext in application crate" -# Enforce that presentation never hand-builds a deps struct (composition-root guard). -# Catches struct literals (`FooDeps { ... }`) and conversions (`FooDeps::from(...)`) — -# both are the composition-root pattern that belongs in crates/composition. -# -# Scope is the whole crate, not just handlers/, so that factoring a -# `fn build_login_deps(state) -> LoginDeps { .. }` helper into a sibling module -# does not slip past the guard. One exclusion: -# src/tests/ — test fixtures legitimately assemble their own state -# `src/main.rs` used to be excluded too ("the binary IS the wiring root") before -# crates/server took over the binary (ADR-0006) — presentation is lib-only now, has no -# main.rs, and the exclusion was dropped rather than left pointing at a file that no -# longer exists. -# NOTE: `\s` is a GNU grep extension, not POSIX ERE. Fine on GNU/Ubuntu runners; -# would need `[[:space:]]` if this ever runs under BusyBox/Alpine grep. check-handler-deps: @if grep -rnE "[A-Za-z0-9_]*Deps(\s*\{|::from\()" crates/presentation/src --include="*.rs" --exclude-dir=tests | grep -q .; then \ echo "❌ hand-built deps struct found in presentation:"; \ @@ -35,30 +19,6 @@ check-handler-deps: fi @echo "✅ No hand-built deps structs in presentation" -# Enforce that presentation calls use cases only, never repository methods directly -# (ADR-0006's guard; ADR-0007 records the widening below). -# -# Was: two shape-matching grep passes for `repos..(` and -# `repos..clone().(`, needed because `AppContext.repos` was still a -# legitimate way to reach a repository — ~40 sites cloned an Arc out of it to pass -# positionally into a use case, so the guard had to distinguish that legal shape -# from an illegal direct call. That shape-matching was demonstrably incomplete: it -# matches `repos..(` textually, so it cannot see a bare reference -# pass (`&repos.diary`, no call at all at the read site) or a `let`-bound access -# (`if let Some(x) = repos.federated_profile` then `x.method()` on the *binding*, -# not on `repos` itself). Both shapes existed in production code — Plan C2's Task 6 -# review caught it, Task 7 fixed both — while the two-pass guard printed a clean -# ✅ the whole time. ADR-0006's "handlers call use cases only" was false at that -# line for as long as the hole existed. -# -# Now: Plan C2 deleted `AppContext.repos` entirely (ADR-0007), so there is no -# longer any legitimate reason for the substring `app_ctx.repos` to appear in -# production code — not a call, not a clone, not a reference, not a let-binding. -# That makes the rule trivial and unevadeable: any mention at all is an offender. -# No shape to match means no shape to miss. -# -# One exclusion: src/tests/ — test helpers still legitimately build a -# `Repositories` and call `composition::build_deps` to produce test state. check-handler-repos: @if grep -rn "app_ctx\.repos" crates/presentation/src --include="*.rs" --exclude-dir=tests | grep -q .; then \ echo "❌ app_ctx.repos referenced in presentation:"; \ @@ -67,18 +27,7 @@ check-handler-repos: fi @echo "✅ No app_ctx.repos references in presentation" -# Enforce that presentation depends on at most one adapter crate. -# -# Presentation may name things that RENDER OUTPUT; it may not name things that -# REACH external systems or storage. `template-askama` is the HTML template -# engine, so it stays. See ADR-0008. -# -# This checks Cargo.toml rather than grepping src/, deliberately: `::` -# greps collide with same-named modules inside `application` and `presentation` -# (`auth::logout` is `application::auth`; `rss::get_user_feed` is -# `handlers::rss`), so the source-level grep cannot decide this. Every directory -# under crates/adapters/ is named exactly like its package, so directory names -# are a safe source of truth. + PRESENTATION_ALLOWED_ADAPTERS := template-askama check-presentation-adapters: @@ -96,41 +45,7 @@ check-presentation-adapters: fi; \ echo "✅ presentation depends on no adapter crate but $(PRESENTATION_ALLOWED_ADAPTERS)" -# Enforce that the federation-off build wires real social behavior, not silent noops. -# -# With federation off, `NoopSocialCommand::follow` returns Ok(()) and writes nothing, -# and `NoopSocialQuery` always answers zero/empty. The write side is currently -# unreachable over HTTP (the social routes are federation-gated in presentation), but -# the query side is not: `get_local_profile`, `get_page_viewer` and `get_activity_feed` -# read it through fully ungated routes, so a noop there means a federation-off -# instance lies about its own follow graph on every profile view and activity feed -# request — contradicting ADR-0003, which established that local follows bypass -# ActivityPub entirely. `application::social::LocalSocialService` is what must be -# wired instead. See ADR-0009. -# -# Two checks, not one: absence of the noops is necessary but not sufficient — deleting -# the whole `#[cfg(not(feature = "federation"))]` wiring block would also make the -# first check pass, leaving federation-off with nothing wired at all, which is worse -# than the noops. So this also asserts `LocalSocialService` is present — and it must -# be present in the federation-OFF region specifically, not merely anywhere in the -# file: `LocalSocialService` is also constructed inside the federation-ON branch -# (feeding `CompositeSocialAdapter`), so a bare file-wide grep is satisfied by that -# occurrence alone and would not catch the federation-off block being deleted. The -# awk carves out the region from the first `#[cfg(not(feature = "federation"))]` to -# the next `#[cfg(feature = "federation")]` — the same kind of range -# `check-presentation-adapters` uses for `[dependencies]` — and greps only inside it. -# Not an occurrence count: that would work today but break the moment the two -# `LocalSocialService` constructions in `main.rs` are deduplicated (a recorded -# follow-up), which could legitimately drop the total to one. -# -# A grep rather than a test because the decision lives in `wire_dependencies()`, a -# private fn in a binary crate, unreachable from integration tests. -# -# Scope is the single file `crates/server/src/main.rs`, not the whole crate: it is the -# sole wiring site for these ports. `worker` and `tui` never name SocialCommand / -# FollowGraphQuery / BlockQuery at all, so there is nowhere else for the noops to -# reappear. Relocating the wiring out of main.rs would move it out of the guard's -# scope — widen this to `crates/server/src` if that ever happens. + check-federation-off-social: @if grep -n "NoopSocialCommand\|NoopSocialQuery" crates/server/src/main.rs | grep -q .; then \ echo "❌ server wires social noops — federation-off would report an empty follow graph:"; \ @@ -143,24 +58,7 @@ check-federation-off-social: fi @echo "✅ server wires real social behavior in every configuration" -# Enforce that `-social` crates never depend on ActivityPub. -# -# ADR-0009's whole federation-optional property rests on one invariant: a -# `-social` crate speaks only `domain::ports`; a `-federation` crate -# speaks ActivityPub; the former must never depend on the latter. It holds today, and -# nothing but this guard enforces it. One manifest line — -# `k-ap = { version = "0.5.0", registry = "gitea" }` in a `-social` crate's Cargo.toml, -# the obvious move when someone wants to reuse a helper like `status_to_str` from -# `sqlite-federation/src/lib.rs` — would silently restore all 76 crates and 19MB that -# federation-off exists to shed, while `make check`, `cargo test` and CI all stay green -# without this guard to catch it. Matches `activitypub` with any suffix, so -# `activitypub_federation = "0.6"` (the upstream crate carrying all 76 transitives) is -# caught too, not just the local `activitypub` crate. -# -# Known gap: a dependency renamed via a `package = "k-ap"` key (`foo = { package = -# "k-ap", ... }`), or declared with a `[dependencies.k-ap]` table header instead of the -# inline form, would not match this line-level regex. Not attempted — nothing in the -# workspace does either today. + check-social-crates-are-ap-free: @if grep -nE '^[[:space:]]*(k-ap|activitypub[A-Za-z0-9_-]*|[A-Za-z0-9_-]+-federation)[[:space:]]*=' crates/adapters/*-social/Cargo.toml | grep -q .; then \ echo "❌ social crate depends on ActivityPub:"; \ @@ -169,23 +67,18 @@ check-social-crates-are-ap-free: fi @echo "✅ social crates depend on no k-ap, activitypub, or *-federation crate" -# Apply rustfmt to all files. fmt: cargo fmt -# Check formatting without modifying files (CI-safe). fmt-check: cargo fmt --check -# Run Clippy and treat warnings as errors. clippy: cargo clippy -- -D warnings -# Run the test suite. test: cargo test -# Apply fmt + clippy auto-fixes in one shot. fix: cargo fmt cargo clippy --fix --allow-dirty --allow-staged diff --git a/crates/adapters/activitypub/src/federation_event_bridge.rs b/crates/adapters/activitypub/src/federation_event_bridge.rs index 90e0270d..2d633f64 100644 --- a/crates/adapters/activitypub/src/federation_event_bridge.rs +++ b/crates/adapters/activitypub/src/federation_event_bridge.rs @@ -64,7 +64,6 @@ impl k_ap::EventPublisher for FederationEventBridge { if let Some(outbox) = outbox_url { tracing::info!(outbox = %outbox, "importing remote outbox after follow accepted"); - // Handled by FollowBackfillHandler reacting to FollowAccepted } Ok(()) } diff --git a/crates/adapters/activitypub/src/federation_ports.rs b/crates/adapters/activitypub/src/federation_ports.rs index 4b4a9fbe..617a498a 100644 --- a/crates/adapters/activitypub/src/federation_ports.rs +++ b/crates/adapters/activitypub/src/federation_ports.rs @@ -9,15 +9,6 @@ use domain::{ use k_ap::ActivityPubService; use uuid::Uuid; -/// Adapts the federation library's service to the three domain-owned ports. -/// -/// A wrapper rather than a bare `impl ... for ActivityPubService` because the -/// orphan rule forbids implementing a foreign trait for a foreign type, and -/// from this crate both `ApDocumentPort` (owned by `domain`) and -/// `ActivityPubService` (owned by the external `k-ap` crate) are foreign. -/// -/// One type carrying all three impls mirrors `CompositeSocialAdapter`, which -/// serves `SocialCommand`, `FollowGraphQuery`, and `BlockQuery` the same way. pub struct ApServiceAdapter { service: Arc, } @@ -28,10 +19,6 @@ impl ApServiceAdapter { } } -/// Single conversion point from the federation library's `anyhow` errors to -/// `DomainError`. The log line and the resulting error string reproduce what -/// `presentation::handlers::social::ap_to_domain` produced before the port -/// inversion moved the boundary here. fn ap_err(e: anyhow::Error) -> DomainError { tracing::error!("ActivityPub error: {:?}", e); DomainError::InfrastructureError(e.to_string()) diff --git a/crates/adapters/activitypub/src/lib.rs b/crates/adapters/activitypub/src/lib.rs index 77bed930..ee8e3531 100644 --- a/crates/adapters/activitypub/src/lib.rs +++ b/crates/adapters/activitypub/src/lib.rs @@ -14,7 +14,6 @@ pub mod watchlist_handler; pub const INSTANCE_ACTOR_ID: uuid::Uuid = uuid::Uuid::from_bytes([0, 0, 0, 0, 0, 0, 0x40, 0, 0x80, 0, 0, 0, 0, 0, 0, 0]); -// Re-export the generic base types that callers need pub use k_ap::{ ActivityPubService, ActivityRepository, ActorRepository, ApContentReader, ApFederationConfig, ApObjectHandler, ApUser, ApUserRepository, BlocklistRepository, FederationData, @@ -41,15 +40,9 @@ pub struct FederationRepos { } pub struct ActivityPubWire { - /// AP document serving. Prefer this over `service` from outside this crate. pub document: std::sync::Arc, - /// Instance domain blocklist. Prefer this over `service` from outside this crate. pub blocklist: std::sync::Arc, - /// Post-follow content backfill. Prefer this over `service` from outside this crate. pub backfill: std::sync::Arc, - /// The concrete service, consumed by `crates/server` to construct - /// `CompositeSocialAdapter`. Everything else should use - /// `document`/`blocklist`/`backfill`. pub service: std::sync::Arc, pub router: axum::Router, pub event_handler: std::sync::Arc, diff --git a/crates/adapters/activitypub/src/urls.rs b/crates/adapters/activitypub/src/urls.rs index 576f0960..8b36a405 100644 --- a/crates/adapters/activitypub/src/urls.rs +++ b/crates/adapters/activitypub/src/urls.rs @@ -1,13 +1,11 @@ use domain::value_objects::{InstanceIdentity, ReviewId, UserId}; use url::Url; -/// Builds the canonical actor URL: `{base_url}/users/{user_id}` pub fn actor_url(instance: &InstanceIdentity, user_id: uuid::Uuid) -> Url { Url::parse(&instance.actor_url_for(&UserId::from_uuid(user_id))) .expect("base_url is always a valid URL prefix") } -/// Builds the canonical review URL: `{base_url}/reviews/{review_id}` pub fn review_url(instance: &InstanceIdentity, review_id: &ReviewId) -> Url { Url::parse(&format!( "{}/reviews/{}", @@ -27,7 +25,6 @@ pub fn goal_url(instance: &InstanceIdentity, user_id: uuid::Uuid, year: u16) -> .expect("base_url is always a valid URL prefix") } -/// Builds the canonical watchlist entry URL: `{base_url}/users/{user_id}/watchlist/{movie_id}` pub fn watchlist_entry_url( instance: &InstanceIdentity, user_id: uuid::Uuid, diff --git a/crates/adapters/adapter-common/src/lib.rs b/crates/adapters/adapter-common/src/lib.rs index e8eb3587..9a0eee62 100644 --- a/crates/adapters/adapter-common/src/lib.rs +++ b/crates/adapters/adapter-common/src/lib.rs @@ -10,31 +10,25 @@ use domain::{ }, }; -/// Map a [`sqlx::Error`] to a [`DomainError::InfrastructureError`], logging the -/// underlying database error at `error` level. pub fn map_sqlx_error(e: sqlx::Error) -> DomainError { tracing::error!("Database error: {:?}", e); DomainError::InfrastructureError("Database operation failed".into()) } -/// Parse a string as a UUID, returning a [`DomainError`] on failure. pub fn parse_uuid(s: &str) -> Result { uuid::Uuid::parse_str(s) .map_err(|e| DomainError::InfrastructureError(format!("Invalid UUID '{}': {}", s, e))) } -/// Parse a `%Y-%m-%d %H:%M:%S` string into a [`chrono::NaiveDateTime`]. pub fn parse_datetime(s: &str) -> Result { chrono::NaiveDateTime::parse_from_str(s, "%Y-%m-%d %H:%M:%S") .map_err(|e| DomainError::InfrastructureError(format!("Invalid datetime '{}': {}", s, e))) } -/// Format a [`chrono::NaiveDateTime`] as `%Y-%m-%d %H:%M:%S`. pub fn datetime_to_str(dt: &chrono::NaiveDateTime) -> String { dt.format("%Y-%m-%d %H:%M:%S").to_string() } -/// Convert a `YYYY-MM` string into a human-readable label like `Jan '24`. pub fn format_year_month(ym: &str) -> String { let parts: Vec<&str> = ym.splitn(2, '-').collect(); if parts.len() != 2 { @@ -59,16 +53,6 @@ pub fn format_year_month(ym: &str) -> String { format!("{} '{}", month, year) } -// --------------------------------------------------------------------------- -// Shared row-to-domain conversion functions -// -// Each database adapter keeps its own `FromRow` structs (sqlite vs postgres -// derive different impls) but the conversion from parsed row fields into -// domain types is identical. These functions capture that shared logic so -// each adapter's `into_domain()` becomes a one-liner delegation. -// --------------------------------------------------------------------------- - -/// Convert raw movie row fields into a [`Movie`] domain object. pub fn movie_row_to_domain( id: String, external_metadata_id: Option, @@ -94,7 +78,6 @@ pub fn movie_row_to_domain( )) } -/// Convert raw review row fields into a [`Review`] domain object. #[allow(clippy::too_many_arguments)] pub fn review_row_to_domain( id: String, @@ -132,10 +115,6 @@ pub fn review_row_to_domain( })) } -/// Assemble a [`MovieSummary`] from an already-converted [`Movie`] and extra -/// metadata fields. The caller is responsible for converting genres into a -/// `Vec` (sqlite splits a comma-separated string, postgres receives a -/// `Vec` directly). pub fn movie_summary_to_domain( movie: Movie, genres: Vec, @@ -154,7 +133,6 @@ pub fn movie_summary_to_domain( } } -/// Convert raw aggregate stats into a [`MovieStats`] domain object. pub fn movie_stats_to_domain( total_count: i64, avg_rating: Option, @@ -175,7 +153,6 @@ pub fn movie_stats_to_domain( } } -/// Convert raw user summary row fields into a [`UserSummary`] domain object. #[allow(clippy::too_many_arguments)] pub fn user_summary_to_domain( id: String, @@ -197,7 +174,6 @@ pub fn user_summary_to_domain( )) } -/// Convert raw watchlist entry fields into a [`WatchlistEntry`] domain object. pub fn watchlist_entry_to_domain( id: String, user_id: String, @@ -212,9 +188,6 @@ pub fn watchlist_entry_to_domain( }) } -/// Convert raw watchlist+movie row fields into a [`WatchlistWithMovie`]. -/// -/// Takes the watchlist entry fields and a pre-converted [`Movie`]. pub fn watchlist_with_movie_to_domain(entry: WatchlistEntry, movie: Movie) -> WatchlistWithMovie { WatchlistWithMovie { entry, movie } } diff --git a/crates/adapters/image-converter/src/backfill.rs b/crates/adapters/image-converter/src/backfill.rs index effebab2..1a814942 100644 --- a/crates/adapters/image-converter/src/backfill.rs +++ b/crates/adapters/image-converter/src/backfill.rs @@ -27,7 +27,7 @@ impl ConversionBackfillJob { #[async_trait] impl PeriodicJob for ConversionBackfillJob { fn interval(&self) -> Duration { - Duration::from_secs(60 * 60 * 24) // 24h + Duration::from_secs(60 * 60 * 24) } async fn run(&self) -> Result<(), DomainError> { diff --git a/crates/adapters/image-converter/src/tests/handler.rs b/crates/adapters/image-converter/src/tests/handler.rs index 66c8a6e9..033a929b 100644 --- a/crates/adapters/image-converter/src/tests/handler.rs +++ b/crates/adapters/image-converter/src/tests/handler.rs @@ -130,8 +130,6 @@ async fn converts_jpeg_to_avif_and_swaps_key() { vec![("avatars/u1".into(), "avatars/u1.avif".into())] ); assert!(storage.get("avatars/u1.avif").await.is_ok()); - // Old raw key deleted — fallback resolves to .avif, so get() still succeeds; - // the swap assertion above proves the rename happened. } #[tokio::test] diff --git a/crates/adapters/importer/src/parsers/xlsx.rs b/crates/adapters/importer/src/parsers/xlsx.rs index 0d5eb09f..dd56dd5d 100644 --- a/crates/adapters/importer/src/parsers/xlsx.rs +++ b/crates/adapters/importer/src/parsers/xlsx.rs @@ -58,14 +58,12 @@ fn cell_to_string(cell: &Data) -> String { Data::Int(i) => i.to_string(), Data::Bool(b) => b.to_string(), Data::DateTime(dt) => { - // ExcelDateTime::to_ymd_hms_milli() works without the chrono feature. let (year, month, day, _, _, _, _) = dt.to_ymd_hms_milli(); format!("{:04}-{:02}-{:02}", year, month, day) } Data::DateTimeIso(s) => s.clone(), Data::DurationIso(s) => s.clone(), Data::Empty | Data::Error(_) => String::new(), - // Fallback for unexpected calamine Data variants; renders as debug string other => format!("{other:?}"), } } diff --git a/crates/adapters/importer/src/tests/mapper.rs b/crates/adapters/importer/src/tests/mapper.rs index 5b14c99a..fec3bb94 100644 --- a/crates/adapters/importer/src/tests/mapper.rs +++ b/crates/adapters/importer/src/tests/mapper.rs @@ -7,7 +7,7 @@ fn sample_file() -> ParsedFile { rows: vec![ vec!["Inception".into(), "10".into(), "2024-01-15".into()], vec!["Dune".into(), "8".into(), "2024-02-20".into()], - vec!["".into(), "3".into(), "2024-03-01".into()], // missing title → invalid + vec!["".into(), "3".into(), "2024-03-01".into()], ], } } @@ -36,10 +36,8 @@ fn full_mappings() -> Vec { fn maps_valid_rows() { let results = apply_mapping(&sample_file(), &full_mappings()); assert_eq!(results.len(), 3); - // First two rows are valid assert!(matches!(results[0].result, RowResult::Valid(_))); assert!(matches!(results[1].result, RowResult::Valid(_))); - // is_duplicate defaults to false assert!(!results[0].is_duplicate); } @@ -47,7 +45,6 @@ fn maps_valid_rows() { fn applies_rating_scale_transform() { let results = apply_mapping(&sample_file(), &full_mappings()); if let RowResult::Valid(row) = &results[0].result { - // 10 * 0.5 = 5 assert_eq!(row.rating.as_deref(), Some("5")); } else { panic!("expected Valid"); @@ -57,7 +54,6 @@ fn applies_rating_scale_transform() { #[test] fn marks_missing_required_fields_invalid() { let results = apply_mapping(&sample_file(), &full_mappings()); - // Row 2 has empty title assert!(matches!(results[2].result, RowResult::Invalid { .. })); } @@ -74,7 +70,6 @@ fn ignores_unmapped_columns() { }; let results = apply_mapping(&file, &mappings); assert_eq!(results.len(), 1); - // Missing rating and watched_at → invalid assert!(matches!(results[0].result, RowResult::Invalid { .. })); } @@ -90,7 +85,6 @@ fn nonexistent_source_column_skipped() { rows: vec![vec!["Inception".into()]], }; let results = apply_mapping(&file, &mappings); - // Column not found → field not set → invalid (missing title, rating, watched_at) assert!(matches!(results[0].result, RowResult::Invalid { .. })); } @@ -107,7 +101,6 @@ fn collects_all_errors_not_just_first() { domain_field: DomainField::Rating, transform: Transform::RatingScale(0.5), }, - // no watched_at mapping ]; let file = ParsedFile { columns: vec!["Name".into(), "Stars".into()], diff --git a/crates/adapters/metadata/src/tmdb.rs b/crates/adapters/metadata/src/tmdb.rs index 7f39f0f5..ec5e776a 100644 --- a/crates/adapters/metadata/src/tmdb.rs +++ b/crates/adapters/metadata/src/tmdb.rs @@ -68,7 +68,7 @@ impl TmdbProvider { struct Details { imdb_id: Option, title: String, - release_date: String, // "YYYY-MM-DD" + release_date: String, poster_path: Option, credits: Credits, } @@ -88,7 +88,6 @@ impl TmdbProvider { )) })?; - // Prefer IMDB ID; fall back to "tmdb:{id}" so the record is still usable. let raw_id = d .imdb_id .filter(|s| !s.is_empty()) diff --git a/crates/adapters/nats/src/consumer.rs b/crates/adapters/nats/src/consumer.rs index 33c4de05..278fc83e 100644 --- a/crates/adapters/nats/src/consumer.rs +++ b/crates/adapters/nats/src/consumer.rs @@ -153,7 +153,6 @@ impl NatsJetStreamConsumer { tracing::debug!("message sent to consumer channel"); } - // messages() stream ended (fetch expired in strict mode) — restart } }); diff --git a/crates/adapters/object-storage/src/lib.rs b/crates/adapters/object-storage/src/lib.rs index d057001b..c183ba10 100644 --- a/crates/adapters/object-storage/src/lib.rs +++ b/crates/adapters/object-storage/src/lib.rs @@ -55,7 +55,6 @@ impl ObjectStorage for ObjectStorageAdapter { Err(DomainError::NotFound(_)) if !has_image_ext(key) => {} Err(e) => return Err(e), } - // Key may reference a pre-conversion path; try converted extensions. for ext in [".webp", ".avif"] { let candidate = format!("{key}{ext}"); if let Ok(bytes) = self.get_exact(&candidate).await { diff --git a/crates/adapters/plex/src/lib.rs b/crates/adapters/plex/src/lib.rs index 4406662f..685b4108 100644 --- a/crates/adapters/plex/src/lib.rs +++ b/crates/adapters/plex/src/lib.rs @@ -4,9 +4,6 @@ use serde::Deserialize; pub struct PlexParser; impl MediaServerParser for PlexParser { - /// Plex sends multipart form data with a `payload` JSON field. - /// The caller must extract the JSON string from the multipart body - /// and pass it here as raw bytes. fn parse_playback_event( &self, body: &[u8], diff --git a/crates/adapters/poster-sync/src/lib.rs b/crates/adapters/poster-sync/src/lib.rs index 2f697711..c7583013 100644 --- a/crates/adapters/poster-sync/src/lib.rs +++ b/crates/adapters/poster-sync/src/lib.rs @@ -116,7 +116,6 @@ impl EventHandler for PosterSyncHandler { movie_id, external_metadata_id, } => { - // Only sync poster if the movie doesn't have one yet let already_has_poster = self .movie_query .get_movie_by_id(&MovieId::from_uuid(movie_id.value())) diff --git a/crates/adapters/postgres-event-queue/src/lib.rs b/crates/adapters/postgres-event-queue/src/lib.rs index cc4004b6..b2e62d7b 100644 --- a/crates/adapters/postgres-event-queue/src/lib.rs +++ b/crates/adapters/postgres-event-queue/src/lib.rs @@ -114,7 +114,6 @@ impl EventConsumer for PostgresEventQueue { return; } } - // no sleep — re-poll immediately when batch was non-empty } } } @@ -140,7 +139,6 @@ async fn claim_batch( pool: &PgPool, config: &DbEventQueueConfig, ) -> Result, DomainError> { - // CTE with FOR UPDATE SKIP LOCKED — atomic and safe for multiple workers let rows = sqlx::query_as::<_, QueueRow>( r#" WITH claimed AS ( diff --git a/crates/adapters/postgres-social/src/lib.rs b/crates/adapters/postgres-social/src/lib.rs index af2d5e6c..02753589 100644 --- a/crates/adapters/postgres-social/src/lib.rs +++ b/crates/adapters/postgres-social/src/lib.rs @@ -11,14 +11,6 @@ pub use remote_goals::PostgresRemoteGoalRepository; use sqlx::PgPool; -/// Postgres-backed implementations of the *domain* social ports. -/// -/// Deliberately separate from `postgres-federation`: this crate knows nothing -/// about ActivityPub, which is what allows a build with the `federation` -/// feature off to exclude the federation stack entirely. See ADR-0009. -/// -/// Shares the `ap_followers` / `ap_following` tables with -/// `postgres-federation`; neither crate owns migrations. pub struct PostgresSocialRepository { pub(crate) pool: PgPool, pub(crate) instance: domain::value_objects::InstanceIdentity, diff --git a/crates/adapters/postgres/src/diary.rs b/crates/adapters/postgres/src/diary.rs index c8c5ee45..62ef34fd 100644 --- a/crates/adapters/postgres/src/diary.rs +++ b/crates/adapters/postgres/src/diary.rs @@ -276,7 +276,6 @@ impl DiaryQuery for PostgresDiaryRepository { let offset = page.offset as i64; let has_search = search.map(|s| !s.is_empty()).unwrap_or(false); - // Dynamic param counter let mut p: i32 = 0; let mut next_param = || { p += 1; @@ -323,9 +322,6 @@ impl DiaryQuery for PostgresDiaryRepository { let where_clause = where_parts.join(" AND "); - // Reset counter for count query (reuse same where_clause string but re-bind) - // We need a separate counter for count SQL — but since where_clause is already built - // with the right $N references, both queries share it. let count_sql = format!( "SELECT COUNT(*) FROM reviews r INNER JOIN movies m ON m.id = r.movie_id @@ -352,7 +348,6 @@ impl DiaryQuery for PostgresDiaryRepository { where_clause, order_clause, limit_param, offset_param ); - // Bind helper closure — binds search + following params in order macro_rules! bind_filter_params { ($q:expr) => {{ let mut q = $q; diff --git a/crates/adapters/postgres/src/insights.rs b/crates/adapters/postgres/src/insights.rs new file mode 100644 index 00000000..a7474e65 --- /dev/null +++ b/crates/adapters/postgres/src/insights.rs @@ -0,0 +1,81 @@ +use std::collections::HashMap; + +use async_trait::async_trait; +use domain::{ + errors::DomainError, models::insights::WatchRow, ports::InsightsQuery, value_objects::UserId, +}; +use sqlx::{PgPool, Row}; + +use adapter_common::{parse_datetime, parse_uuid}; + +use crate::wrapup::fetch_genres_pg; + +/// Postgres twin of `SqliteInsightsQuery` — same shape, `$n` binds, and the +/// same thin "rows in, rows out" contract (ADR-0010). +pub struct PostgresInsightsQuery { + pool: PgPool, +} + +impl PostgresInsightsQuery { + pub fn new(pool: PgPool) -> Self { + Self { pool } + } +} + +#[async_trait] +impl InsightsQuery for PostgresInsightsQuery { + async fn get_watch_rows(&self, user_id: &UserId) -> Result, DomainError> { + let rows = sqlx::query( + "SELECT movie_id, rating, \ + to_char(watched_at AT TIME ZONE 'UTC', 'YYYY-MM-DD HH24:MI:SS') AS watched_at, \ + watch_medium \ + FROM reviews \ + WHERE user_id = $1 \ + ORDER BY watched_at ASC", + ) + .bind(user_id.value().to_string()) + .fetch_all(&self.pool) + .await + .map_err(adapter_common::map_sqlx_error)?; + + let mut movie_ids: Vec = Vec::new(); + let mut seen = std::collections::HashSet::new(); + for row in &rows { + let mid: String = row + .try_get("movie_id") + .map_err(adapter_common::map_sqlx_error)?; + if seen.insert(mid.clone()) { + movie_ids.push(mid); + } + } + + let genres_map: HashMap> = + fetch_genres_pg(&self.pool, &movie_ids).await?; + + let mut result = Vec::with_capacity(rows.len()); + for row in &rows { + let movie_id: String = row + .try_get("movie_id") + .map_err(adapter_common::map_sqlx_error)?; + let rating: i64 = row + .try_get("rating") + .map_err(adapter_common::map_sqlx_error)?; + let watched_at: String = row + .try_get("watched_at") + .map_err(adapter_common::map_sqlx_error)?; + let watch_medium: Option = row + .try_get("watch_medium") + .map_err(adapter_common::map_sqlx_error)?; + + result.push(WatchRow { + movie_id: parse_uuid(&movie_id)?, + rating: rating as u8, + watched_at: parse_datetime(&watched_at)?, + watch_medium, + genres: genres_map.get(&movie_id).cloned().unwrap_or_default(), + }); + } + + Ok(result) + } +} diff --git a/crates/adapters/postgres/src/lib.rs b/crates/adapters/postgres/src/lib.rs index fd14134e..28c2438e 100644 --- a/crates/adapters/postgres/src/lib.rs +++ b/crates/adapters/postgres/src/lib.rs @@ -6,6 +6,7 @@ mod goals; mod image_ref; mod import_profile; mod import_session; +mod insights; mod models; mod movie; mod movie_dedup; @@ -25,6 +26,7 @@ pub use diary::PostgresDiaryRepository; pub use image_ref::{PostgresImageRefAdapter, create_image_ref}; pub use import_profile::PostgresImportProfileRepository; pub use import_session::PostgresImportSessionRepository; +pub use insights::PostgresInsightsQuery; pub use movie::PostgresMovieRepository; pub use movie_dedup::PostgresMovieDeduplicator; pub use persons::{PostgresPersonAdapter, create_person_adapter}; @@ -68,6 +70,7 @@ pub struct PostgresWireOutput { pub ap_content: std::sync::Arc, pub wrapup_repo: std::sync::Arc, pub wrapup_stats: std::sync::Arc, + pub insights: std::sync::Arc, pub goal_command: std::sync::Arc, pub goal_query: std::sync::Arc, pub user_settings: std::sync::Arc, @@ -111,6 +114,7 @@ pub async fn wire(database_url: &str) -> anyhow::Result { ap_content: std::sync::Arc::new(PostgresApContentQuery::new(pool.clone())) as _, wrapup_repo: std::sync::Arc::new(PostgresWrapUpRepository::new(pool.clone())) as _, wrapup_stats: std::sync::Arc::new(PostgresWrapUpStatsQuery::new(pool.clone())) as _, + insights: std::sync::Arc::new(PostgresInsightsQuery::new(pool.clone())) as _, goal_command: std::sync::Arc::new(goals::PostgresGoalRepository::new(pool.clone())) as _, goal_query: std::sync::Arc::new(goals::PostgresGoalRepository::new(pool.clone())) as _, user_settings: std::sync::Arc::clone(&user_settings_repo) as _, diff --git a/crates/adapters/postgres/src/movie_dedup.rs b/crates/adapters/postgres/src/movie_dedup.rs index f796aa0e..8d0d2e89 100644 --- a/crates/adapters/postgres/src/movie_dedup.rs +++ b/crates/adapters/postgres/src/movie_dedup.rs @@ -37,7 +37,6 @@ impl MovieDeduplicator for PostgresMovieDeduplicator { .await .map_err(adapter_common::map_sqlx_error)?; - // 1. Upsert canonical movie record sqlx::query( "INSERT INTO movies (id, external_metadata_id, title, release_year, director, poster_path) VALUES ($1, $2, $3, $4, $5, $6) @@ -48,7 +47,6 @@ impl MovieDeduplicator for PostgresMovieDeduplicator { .bind(&new).bind(&ext_id).bind(&title).bind(year).bind(&director).bind(&poster) .execute(&mut *tx).await.map_err(adapter_common::map_sqlx_error)?; - // 2. Re-point simple FK tables let reviews = sqlx::query("UPDATE reviews SET movie_id = $1 WHERE movie_id = $2") .bind(&new) .bind(&old) @@ -74,7 +72,6 @@ impl MovieDeduplicator for PostgresMovieDeduplicator { .map_err(adapter_common::map_sqlx_error)? .rows_affected(); - // 3. Re-point movie_profiles (PK — move only if canonical has none) let profiles = sqlx::query("UPDATE movie_profiles SET movie_id = $1 WHERE movie_id = $2") .bind(&new) .bind(&old) @@ -83,8 +80,6 @@ impl MovieDeduplicator for PostgresMovieDeduplicator { .map_err(adapter_common::map_sqlx_error)? .rows_affected(); - // 4. Re-point enrichment tables with composite PKs (INSERT … ON CONFLICT DO NOTHING + DELETE) - // Canonical's existing rows win on conflict — old duplicates are discarded. sqlx::query( "INSERT INTO movie_genres (movie_id, tmdb_id, name) SELECT $1, tmdb_id, name FROM movie_genres WHERE movie_id = $2 @@ -139,7 +134,6 @@ impl MovieDeduplicator for PostgresMovieDeduplicator { .await .map_err(adapter_common::map_sqlx_error)?; - // 5. Delete the now-empty old movie record (remaining cascades are safe: all FKs cleared above) sqlx::query("DELETE FROM movies WHERE id = $1") .bind(&old) .execute(&mut *tx) diff --git a/crates/adapters/postgres/src/wrapup.rs b/crates/adapters/postgres/src/wrapup.rs index 44fc6439..ffffda6b 100644 --- a/crates/adapters/postgres/src/wrapup.rs +++ b/crates/adapters/postgres/src/wrapup.rs @@ -414,7 +414,8 @@ struct CastEntry { profile_path: Option, } -async fn fetch_genres_pg( +/// Shared by WrapUp and Insights: movie-id → genre names. +pub(crate) async fn fetch_genres_pg( pool: &PgPool, movie_ids: &[String], ) -> Result>, DomainError> { diff --git a/crates/adapters/sqlite-social/src/lib.rs b/crates/adapters/sqlite-social/src/lib.rs index 1ac48d93..06f3e466 100644 --- a/crates/adapters/sqlite-social/src/lib.rs +++ b/crates/adapters/sqlite-social/src/lib.rs @@ -11,14 +11,6 @@ pub use remote_goals::SqliteRemoteGoalRepository; use sqlx::SqlitePool; -/// SQLite-backed implementations of the *domain* social ports. -/// -/// Deliberately separate from `sqlite-federation`: this crate knows nothing -/// about ActivityPub, which is what allows a build with the `federation` -/// feature off to exclude the federation stack entirely. See ADR-0009. -/// -/// Shares the `ap_followers` / `ap_following` tables with -/// `sqlite-federation`; neither crate owns migrations. pub struct SqliteSocialRepository { pub(crate) pool: SqlitePool, pub(crate) instance: domain::value_objects::InstanceIdentity, diff --git a/crates/adapters/sqlite/src/insights.rs b/crates/adapters/sqlite/src/insights.rs new file mode 100644 index 00000000..6762b851 --- /dev/null +++ b/crates/adapters/sqlite/src/insights.rs @@ -0,0 +1,80 @@ +use std::collections::HashMap; + +use async_trait::async_trait; +use domain::{ + errors::DomainError, models::insights::WatchRow, ports::InsightsQuery, value_objects::UserId, +}; +use sqlx::{Row, SqlitePool}; + +use adapter_common::{parse_datetime, parse_uuid}; + +use crate::wrapup::fetch_genres_sqlite; + +/// SQLite read side of Insights (ADR-0010). Deliberately thin: one query for +/// review basics, one for genres — every threshold and comparison lives in +/// the pure analyzer, not here. +pub struct SqliteInsightsQuery { + pool: SqlitePool, +} + +impl SqliteInsightsQuery { + pub fn new(pool: SqlitePool) -> Self { + Self { pool } + } +} + +#[async_trait] +impl InsightsQuery for SqliteInsightsQuery { + async fn get_watch_rows(&self, user_id: &UserId) -> Result, DomainError> { + let rows = sqlx::query( + "SELECT movie_id, rating, watched_at, watch_medium \ + FROM reviews \ + WHERE user_id = ? \ + ORDER BY watched_at ASC", + ) + .bind(user_id.value().to_string()) + .fetch_all(&self.pool) + .await + .map_err(adapter_common::map_sqlx_error)?; + + let mut movie_ids: Vec = Vec::new(); + let mut seen = std::collections::HashSet::new(); + for row in &rows { + let mid: String = row + .try_get("movie_id") + .map_err(adapter_common::map_sqlx_error)?; + if seen.insert(mid.clone()) { + movie_ids.push(mid); + } + } + + let genres_map: HashMap> = + fetch_genres_sqlite(&self.pool, &movie_ids).await?; + + let mut result = Vec::with_capacity(rows.len()); + for row in &rows { + let movie_id: String = row + .try_get("movie_id") + .map_err(adapter_common::map_sqlx_error)?; + let rating: i64 = row + .try_get("rating") + .map_err(adapter_common::map_sqlx_error)?; + let watched_at: String = row + .try_get("watched_at") + .map_err(adapter_common::map_sqlx_error)?; + let watch_medium: Option = row + .try_get("watch_medium") + .map_err(adapter_common::map_sqlx_error)?; + + result.push(WatchRow { + movie_id: parse_uuid(&movie_id)?, + rating: rating as u8, + watched_at: parse_datetime(&watched_at)?, + watch_medium, + genres: genres_map.get(&movie_id).cloned().unwrap_or_default(), + }); + } + + Ok(result) + } +} diff --git a/crates/adapters/sqlite/src/lib.rs b/crates/adapters/sqlite/src/lib.rs index 8d07876d..05a59f60 100644 --- a/crates/adapters/sqlite/src/lib.rs +++ b/crates/adapters/sqlite/src/lib.rs @@ -5,6 +5,7 @@ mod goals; mod image_ref; mod import_profile; mod import_session; +mod insights; mod migrations; mod models; mod movie; @@ -25,6 +26,7 @@ pub use diary::SqliteDiaryRepository; pub use image_ref::{SqliteImageRefAdapter, create_image_ref}; pub use import_profile::SqliteImportProfileRepository; pub use import_session::SqliteImportSessionRepository; +pub use insights::SqliteInsightsQuery; pub use movie::SqliteMovieRepository; pub use movie_dedup::SqliteMovieDeduplicator; pub use persons::{SqlitePersonAdapter, create_person_adapter}; @@ -64,6 +66,7 @@ pub struct SqliteWireOutput { pub ap_content: std::sync::Arc, pub wrapup_repo: std::sync::Arc, pub wrapup_stats: std::sync::Arc, + pub insights: std::sync::Arc, pub goal_command: std::sync::Arc, pub goal_query: std::sync::Arc, pub user_settings: std::sync::Arc, @@ -114,6 +117,7 @@ pub async fn wire(database_url: &str) -> anyhow::Result { ap_content: std::sync::Arc::new(SqliteApContentQuery::new(pool.clone())) as _, wrapup_repo: std::sync::Arc::new(SqliteWrapUpRepository::new(pool.clone())) as _, wrapup_stats: std::sync::Arc::new(SqliteWrapUpStatsQuery::new(pool.clone())) as _, + insights: std::sync::Arc::new(SqliteInsightsQuery::new(pool.clone())) as _, goal_command: std::sync::Arc::new(goals::SqliteGoalRepository::new(pool.clone())) as _, goal_query: std::sync::Arc::new(goals::SqliteGoalRepository::new(pool.clone())) as _, user_settings: std::sync::Arc::clone(&user_settings_repo) as _, diff --git a/crates/adapters/sqlite/src/models.rs b/crates/adapters/sqlite/src/models.rs index 9a021bc1..186cbaa1 100644 --- a/crates/adapters/sqlite/src/models.rs +++ b/crates/adapters/sqlite/src/models.rs @@ -99,7 +99,6 @@ impl ReviewRow { } } -// Used by query_diary JOIN — r.id aliased to review_id to avoid ambiguity with m.id #[derive(sqlx::FromRow)] pub(crate) struct DiaryRow { pub id: String, @@ -173,7 +172,6 @@ impl MovieStatsRow { } } -// Like DiaryRow but includes user_email from JOIN with users table #[derive(sqlx::FromRow)] pub(crate) struct FeedRow { pub id: String, diff --git a/crates/adapters/sqlite/src/movie_dedup.rs b/crates/adapters/sqlite/src/movie_dedup.rs index ce4ee589..689ca920 100644 --- a/crates/adapters/sqlite/src/movie_dedup.rs +++ b/crates/adapters/sqlite/src/movie_dedup.rs @@ -37,7 +37,6 @@ impl MovieDeduplicator for SqliteMovieDeduplicator { .await .map_err(adapter_common::map_sqlx_error)?; - // 1. Upsert canonical movie record sqlx::query( "INSERT INTO movies (id, external_metadata_id, title, release_year, director, poster_path) VALUES (?, ?, ?, ?, ?, ?) @@ -48,7 +47,6 @@ impl MovieDeduplicator for SqliteMovieDeduplicator { .bind(&new).bind(&ext_id).bind(&title).bind(year).bind(&director).bind(&poster) .execute(&mut *tx).await.map_err(adapter_common::map_sqlx_error)?; - // 2. Re-point simple FK tables let reviews = sqlx::query("UPDATE reviews SET movie_id = ? WHERE movie_id = ?") .bind(&new) .bind(&old) @@ -73,7 +71,6 @@ impl MovieDeduplicator for SqliteMovieDeduplicator { .map_err(adapter_common::map_sqlx_error)? .rows_affected(); - // 3. Re-point movie_profiles (PK — move only if canonical has none) let profiles = sqlx::query("UPDATE movie_profiles SET movie_id = ? WHERE movie_id = ?") .bind(&new) .bind(&old) @@ -82,8 +79,6 @@ impl MovieDeduplicator for SqliteMovieDeduplicator { .map_err(adapter_common::map_sqlx_error)? .rows_affected(); - // 4. Re-point enrichment tables with composite PKs (INSERT OR IGNORE + DELETE) - // Canonical's existing rows win on conflict — old duplicates are discarded. sqlx::query( "INSERT OR IGNORE INTO movie_genres (movie_id, tmdb_id, name) SELECT ?, tmdb_id, name FROM movie_genres WHERE movie_id = ?", @@ -134,7 +129,6 @@ impl MovieDeduplicator for SqliteMovieDeduplicator { .await .map_err(adapter_common::map_sqlx_error)?; - // 5. Delete the now-empty old movie record (remaining cascades are safe: all FKs cleared above) sqlx::query("DELETE FROM movies WHERE id = ?") .bind(&old) .execute(&mut *tx) diff --git a/crates/adapters/sqlite/src/tests/diary.rs b/crates/adapters/sqlite/src/tests/diary.rs index 78c6660d..477beaf6 100644 --- a/crates/adapters/sqlite/src/tests/diary.rs +++ b/crates/adapters/sqlite/src/tests/diary.rs @@ -9,8 +9,6 @@ use sqlx::SqlitePool; async fn setup(pool: &SqlitePool) { sqlx::migrate!("./migrations").run(pool).await.unwrap(); - // carol is a remote actor; we still need a non-null user_id for the schema, - // so we create a local "ghost" user and link the remote review via remote_actor_url. sqlx::query( "INSERT INTO users (id, email, username, password_hash, created_at) VALUES ('11111111-1111-1111-1111-111111111111', 'alice@example.com', 'alice', 'hash', '2024-01-01 00:00:00'), @@ -31,7 +29,6 @@ async fn setup(pool: &SqlitePool) { .await .unwrap(); - // carol's review: local user_id=33333333, remote_actor_url set → remote review sqlx::query( "INSERT INTO reviews (id, movie_id, user_id, rating, watched_at, created_at, remote_actor_url) VALUES ('a1a1a1a1-a1a1-a1a1-a1a1-a1a1a1a1a1a1', 'aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa', '11111111-1111-1111-1111-111111111111', 5, '2024-01-01 00:00:00', '2024-01-01 00:00:00', NULL), @@ -97,7 +94,7 @@ async fn test_following_filter() { .await .unwrap(); - assert_eq!(result.items.len(), 2); // alice + carol, NOT bob + assert_eq!(result.items.len(), 2); let titles: Vec = result .items .iter() @@ -113,7 +110,6 @@ async fn test_get_movie_stats_local() { setup(&pool).await; let repo = SqliteDiaryRepository::new(pool); - // Inception: 1 local review, rating=5, no federated let movie_id = domain::value_objects::MovieId::from_uuid( uuid::Uuid::parse_str("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa").unwrap(), ); @@ -122,8 +118,8 @@ async fn test_get_movie_stats_local() { assert_eq!(stats.total_count, 1); assert_eq!(stats.federated_count, 0); assert!((stats.avg_rating.unwrap() - 5.0).abs() < 0.001); - assert_eq!(stats.rating_histogram[4], 1); // 5★ bucket - assert_eq!(stats.rating_histogram[0], 0); // 1★ bucket + assert_eq!(stats.rating_histogram[4], 1); + assert_eq!(stats.rating_histogram[0], 0); } #[tokio::test] @@ -176,7 +172,6 @@ async fn test_get_movie_social_feed_pagination() { let movie_id = domain::value_objects::MovieId::from_uuid( uuid::Uuid::parse_str("aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa").unwrap(), ); - // offset beyond results: total_count still correct, items empty let page = PageParams::new(Some(10), Some(5)).unwrap(); let result = repo.get_movie_social_feed(&movie_id, &page).await.unwrap(); @@ -190,7 +185,6 @@ async fn test_get_movie_stats_federated() { setup(&pool).await; let repo = SqliteDiaryRepository::new(pool); - // Dune: 1 federated review, rating=4 let movie_id = domain::value_objects::MovieId::from_uuid( uuid::Uuid::parse_str("cccccccc-cccc-cccc-cccc-cccccccccccc").unwrap(), ); @@ -198,8 +192,8 @@ async fn test_get_movie_stats_federated() { assert_eq!(stats.total_count, 1); assert_eq!(stats.federated_count, 1); - assert_eq!(stats.rating_histogram[3], 1); // 4★ bucket - assert_eq!(stats.rating_histogram[4], 0); // 5★ bucket + assert_eq!(stats.rating_histogram[3], 1); + assert_eq!(stats.rating_histogram[4], 0); } #[tokio::test] @@ -208,7 +202,6 @@ async fn count_local_posts_excludes_remote_reviews() { setup(&pool).await; let repo = SqliteDiaryRepository::new(pool); - // setup() seeds 3 reviews: 2 local (alice, bob) + 1 remote (carol) let count = repo.count_local_posts().await.unwrap(); assert_eq!(count, 2); } diff --git a/crates/adapters/sqlite/src/wrapup.rs b/crates/adapters/sqlite/src/wrapup.rs index 2c5b82d1..521c3d1f 100644 --- a/crates/adapters/sqlite/src/wrapup.rs +++ b/crates/adapters/sqlite/src/wrapup.rs @@ -437,7 +437,8 @@ fn in_placeholders(n: usize) -> String { s } -async fn fetch_genres_sqlite( +/// Shared by WrapUp and Insights: movie-id → genre names. +pub(crate) async fn fetch_genres_sqlite( pool: &SqlitePool, movie_ids: &[String], ) -> Result>, DomainError> { diff --git a/crates/api-types/src/diary.rs b/crates/api-types/src/diary.rs index e897a9b7..f428e220 100644 --- a/crates/api-types/src/diary.rs +++ b/crates/api-types/src/diary.rs @@ -60,7 +60,6 @@ pub struct FeedEntryDto { #[derive(Debug, Clone, Deserialize, utoipa::IntoParams)] #[into_params(parameter_in = Query)] pub struct ExportQueryParams { - /// Output format: `csv` (default) or `json` #[serde(default = "default_export_format")] pub format: String, } diff --git a/crates/api-types/src/import.rs b/crates/api-types/src/import.rs index a5ee8873..6cbb0080 100644 --- a/crates/api-types/src/import.rs +++ b/crates/api-types/src/import.rs @@ -17,13 +17,9 @@ pub struct SessionStateResponse { #[derive(Debug, Clone, Serialize, Deserialize, utoipa::ToSchema)] pub struct ApiFieldMapping { - /// Column name in the source file pub source_column: String, - /// Domain field: title | release_year | director | rating | watched_at | comment | external_metadata_id pub domain_field: String, - /// For rating fields: multiply raw value by this factor (e.g. 0.5 for 10-point → 5-point scale) pub rating_scale: Option, - /// For watched_at fields: strftime format hint (e.g. "%d/%m/%Y") pub date_format: Option, } @@ -34,15 +30,12 @@ pub struct ApplyMappingRequest { #[derive(Debug, Clone, Serialize, Deserialize, utoipa::ToSchema)] pub struct ConfirmRequest { - /// Indices (0-based) of rows from the mapping preview to import pub confirmed_indices: Vec, } #[derive(Debug, Clone, Serialize, Deserialize, utoipa::ToSchema)] pub struct SaveProfileRequest { - /// Session UUID whose current field_mappings to save pub session_id: String, - /// Human-readable profile name (e.g. "Letterboxd") pub name: String, } diff --git a/crates/api-types/src/insights.rs b/crates/api-types/src/insights.rs new file mode 100644 index 00000000..45aed22a --- /dev/null +++ b/crates/api-types/src/insights.rs @@ -0,0 +1,49 @@ +use serde::{Deserialize, Serialize}; +use utoipa::ToSchema; + +/// One conservative, baseline-relative statement about the user's viewing +/// behavior. Tagged by `kind` so the SPA can switch on it; every variant +/// carries `sample_count` so the UI can explain a thin sample. +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +#[serde(tag = "kind", rename_all = "snake_case")] +pub enum InsightDto { + /// `genre`'s average rating sits `delta` stars from the user's own + /// average (first-time watches only). + RatingLift { + genre: String, + genre_avg: f64, + user_avg: f64, + delta: f64, + sample_count: u32, + }, + /// `genre`'s share of watches in `month` (1–12) is `multiplier` times the + /// uniform 1/12 baseline. + Seasonality { + genre: String, + month: u32, + multiplier: f64, + sample_count: u32, + }, + /// `medium`'s weekend share of watches is `multiplier` times the user's + /// overall weekend share. + MediumTiming { + medium: String, + multiplier: f64, + weekend_share: f64, + baseline_share: f64, + sample_count: u32, + }, + /// Watching concentrates in a 3-hour window starting `peak_hour_start` + /// (0–23) on `top_days` (Mon=1 … Sun=7). + Rhythm { + peak_hour_start: u32, + peak_hour_end: u32, + top_days: Vec, + sample_count: u32, + }, +} + +#[derive(Debug, Clone, Serialize, Deserialize, ToSchema)] +pub struct InsightsResponse { + pub insights: Vec, +} diff --git a/crates/api-types/src/lib.rs b/crates/api-types/src/lib.rs index 537df34d..7d9319fc 100644 --- a/crates/api-types/src/lib.rs +++ b/crates/api-types/src/lib.rs @@ -3,6 +3,7 @@ pub mod common; pub mod diary; pub mod goals; pub mod import; +pub mod insights; pub mod movies; pub mod rendering; pub mod search; @@ -17,6 +18,7 @@ pub use common::*; pub use diary::*; pub use goals::*; pub use import::*; +pub use insights::*; pub use movies::*; pub use rendering::*; pub use social::*; diff --git a/crates/api-types/src/movies.rs b/crates/api-types/src/movies.rs index 99d3eb8f..a09e1607 100644 --- a/crates/api-types/src/movies.rs +++ b/crates/api-types/src/movies.rs @@ -10,11 +10,8 @@ use crate::common::SocialFeedResponse; pub struct MoviesQueryParams { pub limit: Option, pub offset: Option, - /// Optional title filter (case-insensitive substring match) pub search: Option, - /// Filter by genre name (case-insensitive) pub genre: Option, - /// Filter by ISO language code (e.g. "en") pub language: Option, } diff --git a/crates/api-types/src/search.rs b/crates/api-types/src/search.rs index 31f2e839..75a33b0f 100644 --- a/crates/api-types/src/search.rs +++ b/crates/api-types/src/search.rs @@ -6,21 +6,13 @@ pub use crate::common::{PaginatedMovieHits, PaginatedPersonHits}; #[derive(Debug, Deserialize, IntoParams)] pub struct SearchQueryParams { - /// Free-text query matched across title, cast, crew, genres and keywords. pub q: Option, - /// Filter by genre name (exact match, case-sensitive). pub genre: Option, - /// Filter by release year. pub year: Option, - /// Filter by person ID (UUID). pub person_id: Option, - /// Filter crew results by department (e.g. "Directing", "Writing"). pub department: Option, - /// Filter by original language code (e.g. "en", "fr"). pub language: Option, - /// Max results to return (default 20). pub limit: Option, - /// Offset for pagination (default 0). pub offset: Option, } diff --git a/crates/api-types/src/social.rs b/crates/api-types/src/social.rs index aa3db6af..94af24e0 100644 --- a/crates/api-types/src/social.rs +++ b/crates/api-types/src/social.rs @@ -16,7 +16,6 @@ pub struct RemoteActorDto { pub handle: String, pub display_name: Option, pub url: String, - /// `Some` for local actors, so the SPA can link internally to `/users/{id}`. pub user_id: Option, pub avatar_url: Option, } @@ -79,12 +78,11 @@ pub struct PendingCountResponse { pub count: usize, } +//todo: move out to separate file #[cfg(test)] mod tests { use super::*; - /// Task 7's SPA zod schema parses these exact lowercase literals — - /// a casing or naming drift here breaks the SPA at runtime. #[test] fn follow_state_dto_serializes_to_lowercase_strings() { assert_eq!( diff --git a/crates/api-types/src/users.rs b/crates/api-types/src/users.rs index 320b8a79..17f783c5 100644 --- a/crates/api-types/src/users.rs +++ b/crates/api-types/src/users.rs @@ -23,7 +23,6 @@ pub struct UsersResponse { #[derive(Debug, Clone, Deserialize, utoipa::IntoParams)] #[into_params(parameter_in = Query)] pub struct UserProfileQueryParams { - /// One of: `recent` (default), `ratings`, `history`, `trends` pub view: Option, pub limit: Option, pub offset: Option, diff --git a/crates/application/src/deps.rs b/crates/application/src/deps.rs index 74b9c588..ae943fd6 100644 --- a/crates/application/src/deps.rs +++ b/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, } +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, diff --git a/crates/application/src/diary/get_user_feed.rs b/crates/application/src/diary/get_user_feed.rs index 3a81e814..00ff5467 100644 --- a/crates/application/src/diary/get_user_feed.rs +++ b/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, } diff --git a/crates/application/src/diary/tests/review_logger.rs b/crates/application/src/diary/tests/review_logger.rs index 15ca1ac5..11e0e0b1 100644 --- a/crates/application/src/diary/tests/review_logger.rs +++ b/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] diff --git a/crates/application/src/import/apply_profile.rs b/crates/application/src/import/apply_profile.rs index bb999e0c..d1330d96 100644 --- a/crates/application/src/import/apply_profile.rs +++ b/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, diff --git a/crates/application/src/import/deps.rs b/crates/application/src/import/deps.rs index 54d3ad43..66320339 100644 --- a/crates/application/src/import/deps.rs +++ b/crates/application/src/import/deps.rs @@ -50,11 +50,6 @@ pub struct ListImportProfilesDeps { pub import_profile: Arc, } -/// 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, pub import_session: Arc, diff --git a/crates/application/src/import/get_mapping_stage.rs b/crates/application/src/import/get_mapping_stage.rs index 0a0ecb72..583fa107 100644 --- a/crates/application/src/import/get_mapping_stage.rs +++ b/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 { diff --git a/crates/application/src/import/get_preview_stage.rs b/crates/application/src/import/get_preview_stage.rs index a4735893..f9e6aaf5 100644 --- a/crates/application/src/import/get_preview_stage.rs +++ b/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)` -/// 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, pub rows: Vec, diff --git a/crates/application/src/import/tests/apply_mapping.rs b/crates/application/src/import/tests/apply_mapping.rs index 252cfb71..86d3417a 100644 --- a/crates/application/src/import/tests/apply_mapping.rs +++ b/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, } diff --git a/crates/application/src/insights/deps.rs b/crates/application/src/insights/deps.rs new file mode 100644 index 00000000..626db6f0 --- /dev/null +++ b/crates/application/src/insights/deps.rs @@ -0,0 +1,7 @@ +use std::sync::Arc; + +use domain::ports::InsightsQuery; + +pub struct GetInsightsDeps { + pub insights: Arc, +} diff --git a/crates/application/src/insights/get.rs b/crates/application/src/insights/get.rs new file mode 100644 index 00000000..d831ca2a --- /dev/null +++ b/crates/application/src/insights/get.rs @@ -0,0 +1,17 @@ +//! 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 domain::{errors::DomainError, models::Insight, value_objects::UserId}; + +use crate::insights::deps::GetInsightsDeps; + +pub async fn execute(deps: &GetInsightsDeps, user_id: UserId) -> Result, DomainError> { + let rows = deps.insights.get_watch_rows(&user_id).await?; + Ok(domain::services::insights_analyzer::build_insights(&rows)) +} + +#[cfg(test)] +#[path = "tests/get.rs"] +mod tests; diff --git a/crates/application/src/insights/mod.rs b/crates/application/src/insights/mod.rs new file mode 100644 index 00000000..b604a8c0 --- /dev/null +++ b/crates/application/src/insights/mod.rs @@ -0,0 +1,2 @@ +pub mod deps; +pub mod get; diff --git a/crates/application/src/insights/tests/get.rs b/crates/application/src/insights/tests/get.rs new file mode 100644 index 00000000..5a65e6b3 --- /dev/null +++ b/crates/application/src/insights/tests/get.rs @@ -0,0 +1,93 @@ +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 = (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)).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 = (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)).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)).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, 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())) + .await + .unwrap_err(); + assert!(matches!( + err, + domain::errors::DomainError::InfrastructureError(_) + )); +} diff --git a/crates/application/src/lib.rs b/crates/application/src/lib.rs index 79e4327e..76345c55 100644 --- a/crates/application/src/lib.rs +++ b/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; diff --git a/crates/application/src/movies/discovery_indexer.rs b/crates/application/src/movies/discovery_indexer.rs index 86c16c5e..82819fd3 100644 --- a/crates/application/src/movies/discovery_indexer.rs +++ b/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, search_command: Arc, diff --git a/crates/application/src/movies/enrich_movie.rs b/crates/application/src/movies/enrich_movie.rs index cd2109a8..f95d728c 100644 --- a/crates/application/src/movies/enrich_movie.rs +++ b/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 { let mut seen: HashMap = HashMap::new(); diff --git a/crates/application/src/movies/resolve.rs b/crates/application/src/movies/resolve.rs index 7e7ef3e8..978c1e09 100644 --- a/crates/application/src/movies/resolve.rs +++ b/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, diff --git a/crates/application/src/services.rs b/crates/application/src/services.rs index b0ffd111..5a301972 100644 --- a/crates/application/src/services.rs +++ b/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, diff --git a/crates/application/src/social/local_service.rs b/crates/application/src/social/local_service.rs index 7205d0bd..368e0ffd 100644 --- a/crates/application/src/social/local_service.rs +++ b/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, follow_command: Arc, diff --git a/crates/application/src/tests/services.rs b/crates/application/src/tests/services.rs index b9b09342..17990a55 100644 --- a/crates/application/src/tests/services.rs +++ b/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 = Arc::new(NoopReviewLogger); diff --git a/crates/application/src/users/authorize_admin.rs b/crates/application/src/users/authorize_admin.rs index 37a9b541..bec7f06f 100644 --- a/crates/application/src/users/authorize_admin.rs +++ b/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, diff --git a/crates/application/src/users/deps.rs b/crates/application/src/users/deps.rs index b0dff526..4394599c 100644 --- a/crates/application/src/users/deps.rs +++ b/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, pub diary: Arc, @@ -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, pub diary: Arc, pub social_query: Arc, } -/// 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, pub follow_graph: Arc, @@ -82,9 +73,6 @@ pub struct AuthorizeAdminDeps { pub user: Arc, } -/// `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>, } diff --git a/crates/application/src/users/get_federated_profile.rs b/crates/application/src/users/get_federated_profile.rs index abb1d43a..0b3198d7 100644 --- a/crates/application/src/users/get_federated_profile.rs +++ b/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, diff --git a/crates/application/src/users/get_local_profile.rs b/crates/application/src/users/get_local_profile.rs index c8945fc0..fbdc8b30 100644 --- a/crates/application/src/users/get_local_profile.rs +++ b/crates/application/src/users/get_local_profile.rs @@ -30,10 +30,6 @@ pub struct ProfileIdentity { pub actor_url: String, pub avatar_url: Option, pub banner_url: Option, - /// 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, } diff --git a/crates/application/src/users/get_profile_settings.rs b/crates/application/src/users/get_profile_settings.rs index 4be896b7..eccbf33f 100644 --- a/crates/application/src/users/get_profile_settings.rs +++ b/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, pub avatar_url: Option, pub banner_url: Option, - /// 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, pub fields: Vec<(String, String)>, } diff --git a/crates/application/src/users/tests/get_federated_profile_stats.rs b/crates/application/src/users/tests/get_federated_profile_stats.rs index 54d4db98..ff2d43fd 100644 --- a/crates/application/src/users/tests/get_federated_profile_stats.rs +++ b/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(); diff --git a/crates/application/src/users/tests/get_local_profile.rs b/crates/application/src/users/tests/get_local_profile.rs index fd1a41dc..27e4b9b2 100644 --- a/crates/application/src/users/tests/get_local_profile.rs +++ b/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(), diff --git a/crates/application/src/users/tests/get_page_viewer.rs b/crates/application/src/users/tests/get_page_viewer.rs index e7741880..7667b4b0 100644 --- a/crates/application/src/users/tests/get_page_viewer.rs +++ b/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(); diff --git a/crates/application/src/users/tests/get_profile_settings.rs b/crates/application/src/users/tests/get_profile_settings.rs index 22585589..a5c3a98b 100644 --- a/crates/application/src/users/tests/get_profile_settings.rs +++ b/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(); diff --git a/crates/application/src/users/tests/resolve_username_to_id.rs b/crates/application/src/users/tests/resolve_username_to_id.rs index 4fa41096..05fa48a5 100644 --- a/crates/application/src/users/tests/resolve_username_to_id.rs +++ b/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(); diff --git a/crates/application/src/watchlist/get_watchlist_for_owner.rs b/crates/application/src/watchlist/get_watchlist_for_owner.rs index 31fd32f1..92f32c75 100644 --- a/crates/application/src/watchlist/get_watchlist_for_owner.rs +++ b/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), Remote(Vec), } -/// `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 { 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) diff --git a/crates/application/src/watchlist/tests/get_watchlist_for_owner.rs b/crates/application/src/watchlist/tests/get_watchlist_for_owner.rs index bba6cdd5..72527542 100644 --- a/crates/application/src/watchlist/tests/get_watchlist_for_owner.rs +++ b/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(); diff --git a/crates/application/src/wrapup/get_ready_report.rs b/crates/application/src/wrapup/get_ready_report.rs index a12668f5..fc92654c 100644 --- a/crates/application/src/wrapup/get_ready_report.rs +++ b/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, diff --git a/crates/application/src/wrapup/tests/get_ready_report.rs b/crates/application/src/wrapup/tests/get_ready_report.rs index 0d4a6072..5fc46c62 100644 --- a/crates/application/src/wrapup/tests/get_ready_report.rs +++ b/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(); diff --git a/crates/composition/src/build.rs b/crates/composition/src/build.rs index d9229fc2..98f534cd 100644 --- a/crates/composition/src/build.rs +++ b/crates/composition/src/build.rs @@ -2,8 +2,9 @@ use application::auth::deps::{ LoginDeps, LogoutDeps, RefreshDeps, RegisterAndLoginDeps, RegisterDeps, }; use application::deps::{ - AuthGroup, Deps, DiaryGroup, GoalsGroup, ImportGroup, IntegrationsGroup, MoviesGroup, - PersonGroup, SearchGroup, SocialGroup, UsersGroup, WatchlistGroup, WorkerDeps, WrapupGroup, + AuthGroup, Deps, DiaryGroup, GoalsGroup, ImportGroup, InsightsGroup, IntegrationsGroup, + MoviesGroup, PersonGroup, SearchGroup, SocialGroup, UsersGroup, WatchlistGroup, WorkerDeps, + WrapupGroup, }; use application::diary::deps::{ DeleteReviewDeps, EditReviewDeps, ExportDiaryDeps, GetActivityFeedDeps, GetDiaryDeps, @@ -15,6 +16,7 @@ use application::import::deps::{ DeleteImportProfileDeps, ExecuteImportDeps, GetMappingStageDeps, GetPreviewStageDeps, GetSessionStateDeps, ListImportProfilesDeps, SaveProfileDeps, }; +use application::insights::deps::GetInsightsDeps; use application::integrations::deps::{ ConfirmWatchEventsDeps, DismissWatchEventsDeps, GenerateWebhookTokenDeps, GetWatchQueueDeps, GetWebhookTokensDeps, IngestWatchEventDeps, RevokeWebhookTokenDeps, @@ -175,6 +177,11 @@ pub fn build_deps( import_profile: repos.import_profile.clone(), }, }, + insights: InsightsGroup { + get: GetInsightsDeps { + insights: repos.insights.clone(), + }, + }, integrations: IntegrationsGroup { ingest_watch_event: IngestWatchEventDeps { webhook_token: repos.webhook_token.clone(), diff --git a/crates/composition/src/factory.rs b/crates/composition/src/factory.rs index 3c225b54..6e36146e 100644 --- a/crates/composition/src/factory.rs +++ b/crates/composition/src/factory.rs @@ -32,6 +32,7 @@ pub struct DatabaseOutput { pub ap_content: Arc, pub wrapup_stats: Arc, pub wrapup_repo: Arc, + pub insights: Arc, pub goal_command: Arc, pub goal_query: Arc, pub user_settings: Arc, @@ -91,6 +92,7 @@ pub async fn build_database_adapters( ap_content: w.ap_content, wrapup_stats: w.wrapup_stats, wrapup_repo: w.wrapup_repo, + insights: w.insights, goal_command: w.goal_command, goal_query: w.goal_query, user_settings: w.user_settings, @@ -145,6 +147,7 @@ pub async fn build_database_adapters( ap_content: w.ap_content, wrapup_stats: w.wrapup_stats, wrapup_repo: w.wrapup_repo, + insights: w.insights, goal_command: w.goal_command, goal_query: w.goal_query, user_settings: w.user_settings, diff --git a/crates/composition/src/repositories.rs b/crates/composition/src/repositories.rs index 0333c5c8..b6ed11c5 100644 --- a/crates/composition/src/repositories.rs +++ b/crates/composition/src/repositories.rs @@ -2,12 +2,12 @@ use std::sync::Arc; use domain::ports::{ BlockQuery, DiaryQuery, FederatedProfileQuery, FederationAdminQuery, FollowGraphQuery, - GoalCommand, GoalQuery, ImportProfileRepository, ImportSessionRepository, MovieCommand, - MovieProfileRepository, MovieQuery, PersonCommand, PersonQuery, RefreshSessionRepository, - RemoteGoalRepository, RemoteWatchlistRepository, ReviewRepository, SearchCommand, SearchPort, - SocialCommand, StatsRepository, UserProfileFieldsRepository, UserRepository, - UserSettingsRepository, WatchEventCommand, WatchEventQuery, WatchlistRepository, - WebhookTokenRepository, WrapUpRepository, WrapUpStatsQuery, + GoalCommand, GoalQuery, ImportProfileRepository, ImportSessionRepository, InsightsQuery, + MovieCommand, MovieProfileRepository, MovieQuery, PersonCommand, PersonQuery, + RefreshSessionRepository, RemoteGoalRepository, RemoteWatchlistRepository, ReviewRepository, + SearchCommand, SearchPort, SocialCommand, StatsRepository, UserProfileFieldsRepository, + UserRepository, UserSettingsRepository, WatchEventCommand, WatchEventQuery, + WatchlistRepository, WebhookTokenRepository, WrapUpRepository, WrapUpStatsQuery, }; #[derive(Clone)] @@ -37,6 +37,7 @@ pub struct Repositories { pub federation_admin: Arc, pub wrapup_stats: Arc, pub wrapup_repo: Arc, + pub insights: Arc, pub goal_command: Arc, pub goal_query: Arc, pub user_settings: Arc, diff --git a/crates/composition/src/tests/build.rs b/crates/composition/src/tests/build.rs index 7171cb70..6a506570 100644 --- a/crates/composition/src/tests/build.rs +++ b/crates/composition/src/tests/build.rs @@ -5,13 +5,13 @@ use domain::testing::{ FakeAuthService, FakeDocumentParser, FakeMetadataClient, FakePasswordHasher, FakePersonQuery, FakePosterFetcher, FakeSearchCommand, FakeSearchPort, FakeStatsRepository, InMemoryGoalRepository, InMemoryImportProfileRepository, InMemoryImportSessionRepository, - InMemoryMovieProfileRepository, InMemoryMovieRepository, InMemoryProfileFieldsRepo, - InMemoryRefreshSessionRepository, InMemoryReviewRepository, InMemorySocialRepository, - InMemoryUserRepository, InMemoryUserSettingsRepository, InMemoryWatchEventRepository, - InMemoryWatchlistRepository, InMemoryWebhookTokenRepository, InMemoryWrapUpRepository, - InMemoryWrapUpStatsQuery, NoopEventPublisher, NoopFederationAdminQuery, NoopObjectStorage, - NoopRemoteGoalRepository, NoopRemoteWatchlistRepository, PanicDiaryExporter, - PanicPersonCommand, + InMemoryInsightsQuery, InMemoryMovieProfileRepository, InMemoryMovieRepository, + InMemoryProfileFieldsRepo, InMemoryRefreshSessionRepository, InMemoryReviewRepository, + InMemorySocialRepository, InMemoryUserRepository, InMemoryUserSettingsRepository, + InMemoryWatchEventRepository, InMemoryWatchlistRepository, InMemoryWebhookTokenRepository, + InMemoryWrapUpRepository, InMemoryWrapUpStatsQuery, NoopEventPublisher, + NoopFederationAdminQuery, NoopObjectStorage, NoopRemoteGoalRepository, + NoopRemoteWatchlistRepository, PanicDiaryExporter, PanicPersonCommand, }; use crate::{DatabaseOutput, Repositories, build::build_deps, build::build_worker_deps}; @@ -254,6 +254,7 @@ fn test_repositories() -> Repositories { federation_admin: Arc::new(NoopFederationAdminQuery), wrapup_stats: InMemoryWrapUpStatsQuery::new(), wrapup_repo: InMemoryWrapUpRepository::new(), + insights: InMemoryInsightsQuery::new(), goal_command: Arc::clone(&goals) as _, goal_query: goals as _, user_settings: InMemoryUserSettingsRepository::new(), @@ -312,6 +313,7 @@ fn test_database_output() -> DatabaseOutput { ap_content: Arc::new(NoopLocalApContentQuery), wrapup_stats: InMemoryWrapUpStatsQuery::new(), wrapup_repo: InMemoryWrapUpRepository::new(), + insights: InMemoryInsightsQuery::new(), goal_command: Arc::clone(&goals) as _, goal_query: goals as _, user_settings: Arc::clone(&user_settings) as _, diff --git a/crates/domain/src/models/federation.rs b/crates/domain/src/models/federation.rs index f89a2bc5..0c610acf 100644 --- a/crates/domain/src/models/federation.rs +++ b/crates/domain/src/models/federation.rs @@ -39,10 +39,6 @@ pub struct FederatedProfile { pub banner_url: Option, } -/// A domain-blocklist entry as presented to admin surfaces. Mirrors what the -/// federation adapter can supply; `blocked_at` is a pre-formatted string -/// because that is what the underlying store returns and every consumer -/// renders it verbatim. #[derive(Debug, Clone)] pub struct BlockedDomainInfo { pub domain: String, @@ -50,9 +46,6 @@ pub struct BlockedDomainInfo { pub blocked_at: String, } -/// A remote actor the local instance follows, reduced to the two fields any -/// consumer actually reads. Deliberately narrower than the federation -/// library's actor type — widening it is a decision, not an oversight. #[derive(Debug, Clone)] pub struct FollowedActorInfo { pub url: String, diff --git a/crates/domain/src/models/insights.rs b/crates/domain/src/models/insights.rs new file mode 100644 index 00000000..2ee261d5 --- /dev/null +++ b/crates/domain/src/models/insights.rs @@ -0,0 +1,66 @@ +use chrono::NaiveDateTime; +use uuid::Uuid; + +/// One denormalized row per Review — everything the insight analyzer needs, +/// nothing it doesn't (ADR-0010). Lighter than `WrapUpMovieRow` on purpose: +/// WrapUp's row is a report-era artifact with poster paths, budgets and cast +/// that Insights must not inherit. +#[derive(Clone, Debug)] +pub struct WatchRow { + pub movie_id: Uuid, + pub rating: u8, + pub watched_at: NaiveDateTime, + pub watch_medium: Option, + pub genres: Vec, +} + +/// A single conservative, baseline-relative statement about a user's viewing +/// behavior (see CONTEXT.md "Insight"). Every variant carries the numbers the +/// SPA needs to render the claim plus the `sample_count` behind it, so the UI +/// can show why a category is empty without another round trip. +/// +/// Ordering contract: `build_insights` returns insights grouped by category in +/// a fixed order (rating lift, seasonality, medium timing, rhythm), strongest +/// first within each category, at most `MAX_PER_CATEGORY` per category. +#[derive(Clone, Debug, PartialEq)] +pub enum Insight { + /// The genre's average rating sits `delta` stars away from the user's own + /// average. First-time watches only — a rewatch is chosen because it is + /// already liked, so counting it would conflate choice with genre. + RatingLift { + genre: String, + genre_avg: f64, + user_avg: f64, + delta: f64, + sample_count: u32, + }, + /// The genre's share of watches in one month of the year is `multiplier` + /// times the uniform 1/12 baseline. + Seasonality { + genre: String, + /// 1–12 + month: u32, + multiplier: f64, + sample_count: u32, + }, + /// The medium's weekend share of watches is `multiplier` times the user's + /// overall weekend share. + MediumTiming { + medium: String, + multiplier: f64, + weekend_share: f64, + baseline_share: f64, + sample_count: u32, + }, + /// When watching actually happens: the 3-hour window and the weekdays that + /// carry the most real-timestamped watches. + Rhythm { + /// 0–23, start of the peak 3-hour window. + peak_hour_start: u32, + /// 0–23, exclusive end of the peak window (`start + 3`, mod 24). + peak_hour_end: u32, + /// Chrono weekday numbers, strongest first: Mon=1 … Sun=7. + top_days: Vec, + sample_count: u32, + }, +} diff --git a/crates/domain/src/models/mod.rs b/crates/domain/src/models/mod.rs index a4aff96d..eae18a79 100644 --- a/crates/domain/src/models/mod.rs +++ b/crates/domain/src/models/mod.rs @@ -12,6 +12,7 @@ pub mod goal; pub mod import; pub mod import_profile; pub mod import_session; +pub mod insights; pub mod person; pub mod remote_goal; pub mod remote_watchlist; @@ -24,6 +25,7 @@ pub mod wrapup; pub use enrichment::*; pub use federation::*; pub use feed::*; +pub use insights::*; pub use movie::*; pub use review::*; pub use stats::{ diff --git a/crates/domain/src/models/person.rs b/crates/domain/src/models/person.rs index 36f0314c..34d589b9 100644 --- a/crates/domain/src/models/person.rs +++ b/crates/domain/src/models/person.rs @@ -12,7 +12,6 @@ impl ExternalPersonId { &self.0 } - /// Parse the TMDb numeric ID from "tmdb:12345". Returns None for other formats. pub fn tmdb_id(&self) -> Option { self.0.strip_prefix("tmdb:").and_then(|s| s.parse().ok()) } diff --git a/crates/domain/src/models/review.rs b/crates/domain/src/models/review.rs index 493dd4d1..5c29b0e0 100644 --- a/crates/domain/src/models/review.rs +++ b/crates/domain/src/models/review.rs @@ -28,7 +28,6 @@ pub struct PersistedReview { pub watch_medium: Option, } -/// Partial update for a review. `None` = unchanged, `Some(None)` = clear, `Some(Some(v))` = set. #[derive(Clone, Debug, Default)] pub struct ReviewEdit { pub rating: Option, @@ -133,7 +132,6 @@ impl Review { pub fn watch_medium(&self) -> Option<&WatchMedium> { self.watch_medium.as_ref() } - /// Returns [star1_filled, star2_filled, ..., star5_filled] pub fn stars(&self) -> [bool; 5] { let r = self.rating.value(); [r >= 1, r >= 2, r >= 3, r >= 4, r >= 5] diff --git a/crates/domain/src/models/search.rs b/crates/domain/src/models/search.rs index abcf46da..214b27d6 100644 --- a/crates/domain/src/models/search.rs +++ b/crates/domain/src/models/search.rs @@ -42,13 +42,9 @@ pub struct PersonSearchHit { pub name: String, pub known_for_department: Option, pub profile_path: Option, - /// Top movie titles this person is known for — populated at query time - /// by joining relational tables, never from the index. pub known_for_titles: Vec, } -/// Document submitted to the search index. -/// Add a new variant here to make a new entity type searchable — the port never changes. pub enum IndexableDocument { Movie { id: MovieId, @@ -58,7 +54,6 @@ pub enum IndexableDocument { Person { id: PersonId, person: Box, - // known_for_titles intentionally absent — no reads inside a command flow }, } diff --git a/crates/domain/src/ports/events.rs b/crates/domain/src/ports/events.rs index a7444ed7..99df3530 100644 --- a/crates/domain/src/ports/events.rs +++ b/crates/domain/src/ports/events.rs @@ -11,9 +11,6 @@ pub trait EventPublisher: Send + Sync { } pub trait EventConsumer: Send + Sync { - /// Returns a stream of event envelopes. Each envelope carries a domain event - /// and an ack handle — callers ack after successful dispatch, nack on failure. - /// Implementations decide transport (NATS, DB queue, in-memory channel). fn consume(&self) -> futures::stream::BoxStream<'_, Result>; } diff --git a/crates/domain/src/ports/federation.rs b/crates/domain/src/ports/federation.rs index d2074b1b..6ec92eb4 100644 --- a/crates/domain/src/ports/federation.rs +++ b/crates/domain/src/ports/federation.rs @@ -6,8 +6,6 @@ use crate::{ models::{BlockedDomainInfo, FollowedActorInfo}, }; -/// Serves ActivityPub documents for content negotiation. Presentation calls -/// this when a peer sends `Accept: application/activity+json`. #[async_trait] pub trait ApDocumentPort: Send + Sync { async fn actor_json(&self, user_id: &str) -> Result; @@ -23,8 +21,6 @@ pub trait ApDocumentPort: Send + Sync { ) -> Result; } -/// Instance-wide domain blocklist administration. Presentation calls this from -/// the admin API and the admin HTML pages; nothing outside presentation does. #[async_trait] pub trait InstanceBlocklistPort: Send + Sync { async fn get_blocked_domains(&self) -> Result, DomainError>; @@ -36,8 +32,6 @@ pub trait InstanceBlocklistPort: Send + Sync { async fn remove_blocked_domain(&self, domain: &str) -> Result<(), DomainError>; } -/// Pulling remote content in, and pushing local content out, after a follow -/// is established. Worker-side only — no HTTP handler calls this. #[async_trait] pub trait ApBackfillPort: Send + Sync { async fn get_following( diff --git a/crates/domain/src/ports/images.rs b/crates/domain/src/ports/images.rs index f329f44c..a16fe31b 100644 --- a/crates/domain/src/ports/images.rs +++ b/crates/domain/src/ports/images.rs @@ -4,7 +4,6 @@ use crate::{errors::DomainError, value_objects::PosterUrl}; #[async_trait] pub trait ObjectStorage: Send + Sync { - /// Stores `image_bytes` at `key` and returns the stored key. async fn store(&self, key: &str, image_bytes: &[u8]) -> Result; async fn get(&self, key: &str) -> Result, DomainError>; async fn get_stream( diff --git a/crates/domain/src/ports/insights.rs b/crates/domain/src/ports/insights.rs new file mode 100644 index 00000000..b7abe6aa --- /dev/null +++ b/crates/domain/src/ports/insights.rs @@ -0,0 +1,12 @@ +use async_trait::async_trait; + +use crate::{errors::DomainError, models::insights::WatchRow, value_objects::UserId}; + +/// Read side of Insights (ADR-0010): fetches one denormalized `WatchRow` per +/// review across the user's whole diary. All thresholds and baseline +/// comparisons live in the pure `insights_analyzer`, never in adapters — +/// SQL's job here is "rows in, rows out" and nothing else. +#[async_trait] +pub trait InsightsQuery: Send + Sync { + async fn get_watch_rows(&self, user_id: &UserId) -> Result, DomainError>; +} diff --git a/crates/domain/src/ports/media_server.rs b/crates/domain/src/ports/media_server.rs index 0caa8958..e28e0772 100644 --- a/crates/domain/src/ports/media_server.rs +++ b/crates/domain/src/ports/media_server.rs @@ -12,7 +12,6 @@ pub trait MediaServerParser: Send + Sync { -> Result, DomainError>; } -/// Write port — mutates watch events. #[async_trait] pub trait WatchEventCommand: Send + Sync { async fn save(&self, event: &WatchEvent) -> Result<(), DomainError>; @@ -32,7 +31,6 @@ pub trait WatchEventCommand: Send + Sync { ) -> Result; } -/// Read port — queries watch events. No mutations. #[async_trait] pub trait WatchEventQuery: Send + Sync { async fn list_pending(&self, user_id: &UserId) -> Result, DomainError>; diff --git a/crates/domain/src/ports/mod.rs b/crates/domain/src/ports/mod.rs index 9c90deba..07793697 100644 --- a/crates/domain/src/ports/mod.rs +++ b/crates/domain/src/ports/mod.rs @@ -8,6 +8,7 @@ pub mod goals; pub mod image_fetcher; pub mod images; pub mod import; +pub mod insights; pub mod jobs; pub mod media_server; pub mod movie; @@ -29,6 +30,7 @@ pub use goals::*; pub use image_fetcher::*; pub use images::*; pub use import::*; +pub use insights::*; pub use jobs::*; pub use media_server::*; pub use movie::*; diff --git a/crates/domain/src/ports/movie.rs b/crates/domain/src/ports/movie.rs index 70a0f7a0..5304548c 100644 --- a/crates/domain/src/ports/movie.rs +++ b/crates/domain/src/ports/movie.rs @@ -9,14 +9,12 @@ use crate::{ value_objects::{ExternalMetadataId, MovieId, MovieTitle, PosterUrl, ReleaseYear}, }; -/// Write port — mutates the movies table. #[async_trait] pub trait MovieCommand: Send + Sync { async fn upsert_movie(&self, movie: &Movie) -> Result<(), DomainError>; async fn delete_movie(&self, movie_id: &MovieId) -> Result<(), DomainError>; } -/// Read port — queries movies. No mutations. #[async_trait] pub trait MovieQuery: Send + Sync { async fn get_movie_by_external_id( @@ -42,7 +40,6 @@ pub trait MovieQuery: Send + Sync { page: &PageParams, filter: &MovieFilter, ) -> Result, DomainError>; - /// Returns all movies that have an external_metadata_id set. Used for deduplication. async fn list_movies_with_external_id(&self) -> Result, DomainError>; } @@ -62,14 +59,11 @@ pub trait MetadataClient: Send + Sync { pub trait MovieProfileRepository: Send + Sync { async fn upsert(&self, profile: &MovieProfile) -> Result<(), DomainError>; async fn get_by_movie_id(&self, id: &MovieId) -> Result, DomainError>; - /// Returns (movie_id, external_metadata_id) for movies with no profile or a stale one - /// (enriched_at older than 30 days). async fn list_stale(&self) -> Result, DomainError>; } #[async_trait] pub trait MovieEnrichmentClient: Send + Sync { - /// Resolves an external ID (TMDb or IMDb) and fetches the full movie profile. async fn fetch_profile( &self, movie_id: MovieId, @@ -79,9 +73,6 @@ pub trait MovieEnrichmentClient: Send + Sync { #[async_trait] pub trait MovieDeduplicator: Send + Sync { - /// Atomically re-points all foreign keys (reviews, watchlist entries, movie profiles) - /// from `old_id` to `canonical_id`, upserts the canonical movie record, then deletes - /// the old duplicate. Returns the number of rows re-pointed across all tables. async fn merge_into_canonical( &self, old_id: &MovieId, diff --git a/crates/domain/src/ports/noop.rs b/crates/domain/src/ports/noop.rs index 714147f1..28007f00 100644 --- a/crates/domain/src/ports/noop.rs +++ b/crates/domain/src/ports/noop.rs @@ -7,7 +7,6 @@ use crate::{ // ── NoopRemoteWatchlistRepository ───────────────────────────────────────────── -/// Stub used when federation is disabled — every operation is a no-op. pub struct NoopRemoteWatchlistRepository; #[async_trait] @@ -113,7 +112,6 @@ impl super::BlockQuery for NoopSocialQuery { // ── NoopFederationAdminQuery ───────────────────────────────────────────────── -/// Stub used when federation is disabled — returns empty results. pub struct NoopFederationAdminQuery; #[async_trait] @@ -127,7 +125,6 @@ impl super::FederationAdminQuery for NoopFederationAdminQuery { // ── NoopApDocument ─────────────────────────────────────────────────────────── -/// Stub used when federation is disabled — every operation is a no-op. pub struct NoopApDocument; #[async_trait] @@ -153,7 +150,6 @@ impl super::ApDocumentPort for NoopApDocument { // ── NoopInstanceBlocklist ──────────────────────────────────────────────────── -/// Stub used when federation is disabled — every operation is a no-op. pub struct NoopInstanceBlocklist; #[async_trait] @@ -173,7 +169,6 @@ impl super::InstanceBlocklistPort for NoopInstanceBlocklist { // ── NoopApBackfill ─────────────────────────────────────────────────────────── -/// Stub used when federation is disabled — every operation is a no-op. pub struct NoopApBackfill; #[async_trait] diff --git a/crates/domain/src/ports/person.rs b/crates/domain/src/ports/person.rs index a6f33b7d..9a2943a1 100644 --- a/crates/domain/src/ports/person.rs +++ b/crates/domain/src/ports/person.rs @@ -10,13 +10,9 @@ pub trait PersonEnrichmentClient: Send + Sync { async fn fetch_details(&self, external_id: &str) -> Result; } -/// Write port — mutates the persons table. No reads. #[async_trait] pub trait PersonCommand: Send + Sync { - /// Upsert a batch of persons. Uses INSERT OR REPLACE (SQLite) / ON CONFLICT DO UPDATE (Postgres). async fn upsert_batch(&self, persons: &[Person]) -> Result<(), DomainError>; - /// Insert a batch of missing persons from movie_cast/movie_crew into the persons table. - /// Returns (inserted_count, has_more). async fn backfill_from_credits_batch( &self, batch_size: u32, @@ -28,7 +24,6 @@ pub trait PersonCommand: Send + Sync { ) -> Result<(), DomainError>; } -/// Read port — queries persons and credits. No mutations. #[async_trait] pub trait PersonQuery: Send + Sync { async fn get_by_id(&self, id: &PersonId) -> Result, DomainError>; @@ -36,10 +31,7 @@ pub trait PersonQuery: Send + Sync { &self, id: &ExternalPersonId, ) -> Result, DomainError>; - /// Returns the person's full cast and crew credit history across all indexed movies. async fn get_credits(&self, id: &PersonId) -> Result; - /// Returns persons who have no remaining entries in movie_cast or movie_crew. - /// Called after movie deletion to find index entries that can be pruned. async fn list_orphaned_persons(&self) -> Result, DomainError>; async fn list_page(&self, limit: u32, offset: u32) -> Result, DomainError>; } diff --git a/crates/domain/src/ports/search.rs b/crates/domain/src/ports/search.rs index b482e802..5ed08957 100644 --- a/crates/domain/src/ports/search.rs +++ b/crates/domain/src/ports/search.rs @@ -5,17 +5,13 @@ use crate::{ models::{EntityType, IndexableDocument, SearchQuery, SearchResults}, }; -/// Read port — executes search queries. No mutations. #[async_trait] pub trait SearchPort: Send + Sync { async fn search(&self, query: &SearchQuery) -> Result; } -/// Write port — manages the search index. No reads. #[async_trait] pub trait SearchCommand: Send + Sync { - /// Add or replace a document in the search index. async fn index(&self, doc: IndexableDocument) -> Result<(), DomainError>; - /// Remove a document from the search index by entity type and internal ID string. async fn remove(&self, entity_type: EntityType, id: &str) -> Result<(), DomainError>; } diff --git a/crates/domain/src/ports/social.rs b/crates/domain/src/ports/social.rs index f2681233..6680e9f6 100644 --- a/crates/domain/src/ports/social.rs +++ b/crates/domain/src/ports/social.rs @@ -127,35 +127,11 @@ pub trait LocalApContentQuery: Send + Sync { ) -> Result, DomainError>; } -/// Resolves a `FollowTarget` (a handle or an already-known identity) to the -/// `SocialIdentity` that should be dispatched on — local vs. remote. -/// -/// Split out from `SocialCommand::follow` because the decision must happen -/// once, before dispatch, and both `LocalSocialService` and -/// `CompositeSocialAdapter` need to observe its result rather than each -/// re-deriving it (which would need `UserRepository` on the composite and -/// duplicate the local/remote fallthrough logic). #[async_trait] pub trait FollowTargetResolver: Send + Sync { async fn resolve_target(&self, target: &FollowTarget) -> Result; } -/// Performs the local follow write for a target whose `SocialIdentity` a caller -/// has *already* resolved via `FollowTargetResolver::resolve_target`. -/// -/// Exists so `CompositeSocialAdapter` — which must call `resolve_target` first -/// to decide local vs. remote dispatch — doesn't then hand the raw -/// `FollowTarget` to `SocialCommand::follow` and pay for a second resolution of -/// the same handle. That second resolution wasn't just wasteful: on a -/// federation-ON deployment it could also change the answer (the target user -/// deleted between the two calls, e.g.), turning a local follow into a bogus -/// "federation is not enabled" error. -/// -/// `LocalSocialService::follow` is `resolve_target_identity` then -/// `follow_resolved` — the self-follow guard and the two-sided write live only -/// in the latter, so a caller that already has the identity (the composite) -/// and one that doesn't (federation-off `SocialCommand::follow`, wired -/// directly) both end up running the exact same write path. #[async_trait] pub trait ResolvedFollow: Send + Sync { async fn follow_resolved( @@ -165,15 +141,6 @@ pub trait ResolvedFollow: Send + Sync { ) -> Result<(), DomainError>; } -/// The subset of social behavior that needs no ActivityPub — everything a -/// single-instance deployment can do with only its own database. -/// -/// A marker supertrait with a blanket impl, so any type implementing all five -/// parts is usable as `Arc` without a separate registration. -/// Same shape as `k_ap::FollowRepository` over its five follow traits. -/// -/// `CompositeSocialAdapter` takes one of these for its local branches, which is -/// what lets the `activitypub` crate stay unaware of `application`. pub trait LocalSocial: SocialCommand + FollowGraphQuery + BlockQuery + FollowTargetResolver + ResolvedFollow { @@ -333,8 +300,6 @@ mod local_social_tests { } } - /// The blanket impl must make any type implementing all five usable as - /// `Arc`, and `LocalSocial` must stay object-safe. #[test] fn blanket_impl_yields_a_trait_object() { let local: Arc = Arc::new(Stub); diff --git a/crates/domain/src/ports/watchlist.rs b/crates/domain/src/ports/watchlist.rs index ecf7ee59..ff87adc4 100644 --- a/crates/domain/src/ports/watchlist.rs +++ b/crates/domain/src/ports/watchlist.rs @@ -11,13 +11,10 @@ use crate::{ #[async_trait] pub trait WatchlistRepository: Send + Sync { - /// Add a new entry. Silently succeeds if the entry already exists. async fn add(&self, entry: &WatchlistEntry) -> Result<(), DomainError>; - /// Remove an entry. Returns NotFound if the entry does not exist. async fn remove(&self, user_id: &UserId, movie_id: &MovieId) -> Result<(), DomainError>; - /// Remove an entry if it exists. Never returns NotFound. async fn remove_if_present( &self, user_id: &UserId, diff --git a/crates/domain/src/services/insights_analyzer.rs b/crates/domain/src/services/insights_analyzer.rs new file mode 100644 index 00000000..04ee8a2e --- /dev/null +++ b/crates/domain/src/services/insights_analyzer.rs @@ -0,0 +1,343 @@ +//! Conservative insight extraction over a user's whole diary (ADR-0010). +//! +//! Every threshold lives here, not in adapters, so the honesty rules are +//! unit-testable without a database. An insight that cannot clear its +//! thresholds is silently absent — an empty page is the correct rendering of +//! a small or noisy diary, not a bug to fix by lowering thresholds. + +use std::collections::HashMap; + +use chrono::{Datelike, Timelike, Weekday}; + +use crate::models::{Insight, WatchRow}; + +// ── Honesty thresholds ─────────────────────────────────────────────────────── + +/// First-time ratings a genre needs before its average means anything. +const MIN_GENRE_RATINGS: usize = 5; +/// First-time ratings overall before the user's own average is a stable baseline. +const MIN_BASELINE_RATINGS: usize = 10; +/// Minimum distance from the user's average before a rating lift is worth saying. +const MIN_RATING_DELTA: f64 = 0.3; +/// Rows a genre needs before month-of-year distribution is more than noise. +const MIN_GENRE_SEASON_ROWS: usize = 15; +/// Minimum lift over the uniform 1/12 month baseline. +const MIN_SEASON_MULTIPLIER: f64 = 1.5; +/// Rows a medium needs before its weekend share means anything. +const MIN_MEDIUM_ROWS: usize = 5; +/// Minimum lift over the user's own weekend share. +const MIN_MEDIUM_MULTIPLIER: f64 = 1.5; +/// More than this share of rows without a medium → the medium-timing category +/// would describe half a diary; skip it entirely. +const MAX_MISSING_MEDIUM_SHARE: f64 = 0.3; +/// Real-timestamped rows needed to say anything about rhythm. +const MIN_RHYTHM_ROWS: usize = 10; +/// More than this share of rows at exactly 00:00 → timestamps are import noise +/// (CSV imports carry no time); skip rhythm entirely. +const MAX_MIDNIGHT_SHARE: f64 = 0.5; +/// Insights shown per category — more than this and the page is a wall. +const MAX_PER_CATEGORY: usize = 3; +/// Width of the peak-hour window in the rhythm insight. +const RHYTHM_WINDOW_HOURS: u32 = 3; + +pub fn build_insights(rows: &[WatchRow]) -> Vec { + let mut insights = Vec::new(); + insights.extend(rating_lifts(rows)); + insights.extend(seasonalities(rows)); + insights.extend(medium_timings(rows)); + insights.extend(rhythm(rows)); + insights +} + +// ── Rating lift ────────────────────────────────────────────────────────────── + +fn rating_lifts(rows: &[WatchRow]) -> Vec { + let first_time = first_time_rows(rows); + if first_time.len() < MIN_BASELINE_RATINGS { + return vec![]; + } + let user_avg = avg_rating(&first_time); + + let mut by_genre: HashMap<&str, Vec> = HashMap::new(); + for row in first_time { + for genre in &row.genres { + by_genre.entry(genre).or_default().push(row.rating); + } + } + + let mut candidates: Vec = by_genre + .into_iter() + .filter(|(_, ratings)| ratings.len() >= MIN_GENRE_RATINGS) + .filter_map(|(genre, ratings)| { + let genre_avg = + ratings.iter().map(|&r| f64::from(r)).sum::() / ratings.len() as f64; + let delta = round2(genre_avg - user_avg); + if delta.abs() < MIN_RATING_DELTA { + return None; + } + Some(( + strength(delta.abs(), ratings.len()), + genre, + genre_avg, + delta, + ratings.len(), + )) + }) + .map(|(_, genre, genre_avg, delta, n)| Insight::RatingLift { + genre: genre.to_string(), + genre_avg: round2(genre_avg), + user_avg: round2(user_avg), + delta, + sample_count: n as u32, + }) + .collect(); + + candidates.sort_by(strongest_first); + candidates.truncate(MAX_PER_CATEGORY); + candidates +} + +/// Strongest first; ties broken by label ascending so output is deterministic. +fn strongest_first(a: &Insight, b: &Insight) -> std::cmp::Ordering { + strength_of(b) + .partial_cmp(&strength_of(a)) + .unwrap_or(std::cmp::Ordering::Equal) + .then_with(|| label_of(a).cmp(label_of(b))) +} + +/// The earliest review of each movie; later reviews of the same movie are +/// rewatches (the same definition `ReviewHistory` uses). +fn first_time_rows(rows: &[WatchRow]) -> Vec<&WatchRow> { + let mut seen: HashMap = HashMap::new(); + for row in rows { + match seen.get(&row.movie_id) { + Some(existing) if existing.watched_at <= row.watched_at => {} + _ => { + seen.insert(row.movie_id, row); + } + } + } + let mut first: Vec<&WatchRow> = seen.into_values().collect(); + first.sort_by_key(|r| r.watched_at); + first +} + +fn avg_rating(rows: &[&WatchRow]) -> f64 { + rows.iter().map(|r| f64::from(r.rating)).sum::() / rows.len() as f64 +} + +// ── Seasonality ────────────────────────────────────────────────────────────── + +fn seasonalities(rows: &[WatchRow]) -> Vec { + let mut by_genre: HashMap<&str, Vec> = HashMap::new(); + for row in rows { + for genre in &row.genres { + by_genre + .entry(genre) + .or_default() + .push(row.watched_at.month()); + } + } + + let mut candidates: Vec = by_genre + .into_iter() + .filter(|(_, months)| months.len() >= MIN_GENRE_SEASON_ROWS) + .filter_map(|(genre, months)| { + let total = months.len() as f64; + let mut histogram = [0u32; 12]; + for &m in &months { + histogram[(m - 1) as usize] += 1; + } + // Smallest month number wins ties — deterministic output. + let (peak_month, peak_count) = histogram + .into_iter() + .enumerate() + .max_by_key(|(i, c)| (*c, std::cmp::Reverse(*i))) + .map(|(i, c)| (i as u32 + 1, c))?; + // Baseline is a uniform year: each month expects total/12 watches. + let multiplier = round2(f64::from(peak_count) * 12.0 / total); + if multiplier < MIN_SEASON_MULTIPLIER { + return None; + } + Some(( + strength(multiplier - 1.0, months.len()), + genre, + peak_month, + multiplier, + months.len(), + )) + }) + .map(|(_, genre, month, multiplier, n)| Insight::Seasonality { + genre: genre.to_string(), + month, + multiplier, + sample_count: n as u32, + }) + .collect(); + + candidates.sort_by(strongest_first); + candidates.truncate(MAX_PER_CATEGORY); + candidates +} + +// ── Medium timing ──────────────────────────────────────────────────────────── + +fn medium_timings(rows: &[WatchRow]) -> Vec { + if rows.is_empty() { + return vec![]; + } + let with_medium_count = rows.iter().filter(|r| r.watch_medium.is_some()).count(); + let missing_share = 1.0 - with_medium_count as f64 / rows.len() as f64; + if missing_share > MAX_MISSING_MEDIUM_SHARE { + return vec![]; + } + + // (total watches, weekend watches) per medium. + let mut by_medium: HashMap<&str, (usize, usize)> = HashMap::new(); + let mut weekend_total = 0usize; + for row in rows.iter().filter(|r| r.watch_medium.is_some()) { + let weekend = matches!(row.watched_at.weekday(), Weekday::Sat | Weekday::Sun); + weekend_total += usize::from(weekend); + let entry = by_medium + .entry(row.watch_medium.as_deref().expect("filtered above")) + .or_insert((0, 0)); + entry.0 += 1; + entry.1 += usize::from(weekend); + } + + let baseline_share = weekend_total as f64 / with_medium_count as f64; + // No weekend watches at all → no baseline to lift against. + if baseline_share == 0.0 { + return vec![]; + } + + let mut candidates: Vec = by_medium + .into_iter() + .filter(|(_, (count, _))| *count >= MIN_MEDIUM_ROWS) + .filter_map(|(medium, (count, weekend))| { + let weekend_share = weekend as f64 / count as f64; + let multiplier = round2(weekend_share / baseline_share); + if multiplier < MIN_MEDIUM_MULTIPLIER { + return None; + } + Some(( + strength(multiplier - 1.0, count), + medium, + weekend_share, + multiplier, + count, + )) + }) + .map( + |(_, medium, weekend_share, multiplier, n)| Insight::MediumTiming { + medium: medium.to_string(), + multiplier, + weekend_share: round2(weekend_share), + baseline_share: round2(baseline_share), + sample_count: n as u32, + }, + ) + .collect(); + + candidates.sort_by(strongest_first); + candidates.truncate(MAX_PER_CATEGORY); + candidates +} + +// ── Rhythm ─────────────────────────────────────────────────────────────────── + +fn rhythm(rows: &[WatchRow]) -> Vec { + let real: Vec<&WatchRow> = rows + .iter() + .filter(|r| !(r.watched_at.hour() == 0 && r.watched_at.minute() == 0)) + .collect(); + let midnight_share = 1.0 - real.len() as f64 / rows.len().max(1) as f64; + if rows.is_empty() || real.len() < MIN_RHYTHM_ROWS || midnight_share > MAX_MIDNIGHT_SHARE { + return vec![]; + } + + let mut hours = [0u32; 24]; + let mut days = [0u32; 7]; // index 0 = Monday (chrono weekday number − 1) + for row in &real { + hours[row.watched_at.hour() as usize] += 1; + days[row.watched_at.weekday().number_from_monday() as usize - 1] += 1; + } + + // Circular 3-hour window with the most watches; smallest start hour wins ties. + let mut best_start = 0usize; + let mut best_count = 0u32; + for start in 0..24 { + let count: u32 = (0..RHYTHM_WINDOW_HOURS) + .map(|o| hours[(start + o as usize) % 24]) + .sum(); + if count > best_count { + best_count = count; + best_start = start; + } + } + + // Days carrying at least 60% of the busiest day's load, strongest first, + // capped at three — "mostly Tue–Thu" without the tail of noise. + let max_day = days.iter().copied().max().unwrap_or(0); + let mut top_days: Vec<(u32, u32)> = days + .into_iter() + .enumerate() + .filter(|(_, c)| *c > 0 && f64::from(*c) >= f64::from(max_day) * 0.6) + .map(|(i, c)| (i as u32 + 1, c)) + .collect(); + // Count descending, then day number ascending — deterministic. + top_days.sort_unstable_by_key(|&(day, count)| (std::cmp::Reverse(count), day)); + let top_days: Vec = top_days.into_iter().map(|(day, _)| day).take(3).collect(); + + vec![Insight::Rhythm { + peak_hour_start: best_start as u32, + peak_hour_end: (best_start as u32 + RHYTHM_WINDOW_HOURS) % 24, + top_days, + sample_count: real.len() as u32, + }] +} + +// ── Shared helpers ─────────────────────────────────────────────────────────── + +/// Strength = effect size scaled by evidence volume. A 1.6× lift on 60 rows +/// outranks 1.7× on 3; a raw lift would not. +fn strength(effect: f64, sample: usize) -> f64 { + effect * (1.0 + (sample as f64).ln()) +} + +fn strength_of(insight: &Insight) -> f64 { + match insight { + Insight::RatingLift { + delta, + sample_count, + .. + } => strength(delta.abs(), *sample_count as usize), + Insight::Seasonality { + multiplier, + sample_count, + .. + } + | Insight::MediumTiming { + multiplier, + sample_count, + .. + } => strength(multiplier - 1.0, *sample_count as usize), + Insight::Rhythm { .. } => 0.0, + } +} + +fn label_of(insight: &Insight) -> &str { + match insight { + Insight::RatingLift { genre, .. } | Insight::Seasonality { genre, .. } => genre, + Insight::MediumTiming { medium, .. } => medium, + Insight::Rhythm { .. } => "", + } +} + +fn round2(v: f64) -> f64 { + (v * 100.0).round() / 100.0 +} + +#[cfg(test)] +#[path = "tests/insights_analyzer.rs"] +mod tests; diff --git a/crates/domain/src/services/mod.rs b/crates/domain/src/services/mod.rs index 398e2047..92dac746 100644 --- a/crates/domain/src/services/mod.rs +++ b/crates/domain/src/services/mod.rs @@ -1,2 +1,3 @@ +pub mod insights_analyzer; pub mod review_history; pub mod wrapup_analyzer; diff --git a/crates/domain/src/services/tests/insights_analyzer.rs b/crates/domain/src/services/tests/insights_analyzer.rs new file mode 100644 index 00000000..4a9595b0 --- /dev/null +++ b/crates/domain/src/services/tests/insights_analyzer.rs @@ -0,0 +1,365 @@ +use chrono::{Datelike, NaiveDate, Weekday}; +use uuid::Uuid; + +use crate::models::{Insight, WatchRow}; + +use super::super::insights_analyzer::build_insights; + +fn watch(genres: &[&str], rating: u8, date: NaiveDate, hour: u32) -> WatchRow { + WatchRow { + movie_id: Uuid::new_v4(), + rating, + watched_at: date.and_hms_opt(hour, 30, 0).unwrap(), + watch_medium: Some("streaming".to_string()), + genres: genres.iter().map(|g| g.to_string()).collect(), + } +} + +fn date(year: i32, month: u32, day: u32) -> NaiveDate { + NaiveDate::from_ymd_opt(year, month, day).unwrap() +} + +fn kinds(insights: &[Insight]) -> Vec<&str> { + insights + .iter() + .map(|i| match i { + Insight::RatingLift { .. } => "rating_lift", + Insight::Seasonality { .. } => "seasonality", + Insight::MediumTiming { .. } => "medium_timing", + Insight::Rhythm { .. } => "rhythm", + }) + .collect() +} + +// ── Rating lift ────────────────────────────────────────────────────────────── + +#[test] +fn rating_lift_compares_genre_to_personal_average() { + // 12 thrillers rated 5, 12 comedies rated 2 — user average 3.5. + let mut rows: Vec = (0..12) + .map(|i| watch(&["Thriller"], 5, date(2024, 1, 1 + i), 20)) + .collect(); + rows.extend((0..12).map(|i| watch(&["Comedy"], 2, date(2024, 3, 1 + i), 20))); + + let insights = build_insights(&rows); + let thrill = insights.iter().find_map(|i| match i { + Insight::RatingLift { + genre, + delta, + sample_count, + .. + } if genre == "Thriller" => Some((*delta, *sample_count)), + _ => None, + }); + let (delta, count) = thrill.expect("Thriller lift expected"); + assert!((delta - 1.5).abs() < 1e-9, "delta was {delta}"); + assert_eq!(count, 12); + + let comedy = insights.iter().find_map(|i| match i { + Insight::RatingLift { genre, delta, .. } if genre == "Comedy" => Some(*delta), + _ => None, + }); + assert!((comedy.expect("Comedy lift expected") + 1.5).abs() < 1e-9); +} + +#[test] +fn rating_lift_ignores_rewatches() { + // 10 first-time dramas rated 3 (baseline), 5 thrillers rated 5… + let mut rows: Vec = (0..10) + .map(|i| watch(&["Drama"], 3, date(2024, 1, 1 + i), 20)) + .collect(); + let mut thriller_ids: Vec = Vec::new(); + for i in 0..5 { + let row = watch(&["Thriller"], 5, date(2024, 2, 1 + i), 20); + thriller_ids.push(row.movie_id); + rows.push(row); + } + // …and 3 rewatches of those same thrillers rated 1, which must not count. + for (i, id) in thriller_ids.iter().enumerate() { + let mut rewatch = watch(&["Thriller"], 1, date(2024, 6, 1 + i as u32), 20); + rewatch.movie_id = *id; + rows.push(rewatch); + } + + let insights = build_insights(&rows); + let thrill = insights.iter().find_map(|i| match i { + Insight::RatingLift { + genre, + delta, + sample_count, + .. + } if genre == "Thriller" => Some((*delta, *sample_count)), + _ => None, + }); + let (delta, count) = thrill.expect("Thriller lift expected"); + // 15 first-time rows: user_avg = (10*3 + 5*5)/15 = 55/15 ≈ 3.67, + // genre_avg = 5.0, delta ≈ 1.33. + assert!((delta - 1.33).abs() < 0.01, "delta was {delta}"); + assert_eq!(count, 5, "rewatches must not join the genre sample"); +} + +#[test] +fn rating_lift_below_sample_threshold_is_absent() { + let mut rows: Vec = (0..10) + .map(|i| watch(&["Drama"], 3, date(2024, 1, 1 + i), 20)) + .collect(); + // Only 4 thriller ratings — below MIN_GENRE_RATINGS, so no matter how + // high they are rated, the genre must not yield an insight. + rows.extend((0..4).map(|i| watch(&["Thriller"], 5, date(2024, 2, 1 + i), 20))); + + let insights = build_insights(&rows); + assert!( + !insights + .iter() + .any(|i| matches!(i, Insight::RatingLift { genre, .. } if genre == "Thriller")), + "a genre with 4 ratings must not yield a lift" + ); +} + +#[test] +fn rating_lift_below_delta_threshold_is_absent() { + // 5 thrillers rated 4; dramas mix to put the personal average at 3.85, + // so the thriller delta is 0.15 < 0.3. + let mut rows: Vec = (0..12) + .map(|i| watch(&["Drama"], 4, date(2024, 1, 1 + i % 28), 20)) + .collect(); + rows.extend((0..3).map(|i| watch(&["Drama"], 3, date(2024, 1, 13 + i), 20))); + rows.extend((0..5).map(|i| watch(&["Thriller"], 4, date(2024, 2, 1 + i), 20))); + + let insights = build_insights(&rows); + assert!(!kinds(&insights).contains(&"rating_lift")); +} + +// ── Seasonality ────────────────────────────────────────────────────────────── + +#[test] +fn seasonality_detects_month_peak_against_uniform_baseline() { + // 16 horrors: 10 in October across two years, 6 scattered. + let mut rows: Vec = Vec::new(); + for i in 0..10u32 { + let year = if i < 5 { 2023 } else { 2024 }; + rows.push(watch(&["Horror"], 4, date(year, 10, 1 + i % 27), 21)); + } + for (m, d) in [(1u32, 3u32), (2, 7), (4, 11), (6, 15), (8, 19), (12, 21)] { + rows.push(watch(&["Horror"], 4, date(2024, m, d), 21)); + } + // A perfectly spread genre (2 watches per month) must NOT appear. + rows.extend((0..24).map(|i| watch(&["Drama"], 3, date(2024, 1 + i % 12, 5), 21))); + + let insights = build_insights(&rows); + let horror = insights.iter().find_map(|i| match i { + Insight::Seasonality { + genre, + month, + multiplier, + sample_count, + .. + } if genre == "Horror" => Some((*month, *multiplier, *sample_count)), + _ => None, + }); + let (month, multiplier, count) = horror.expect("Horror seasonality expected"); + assert_eq!(month, 10); + // 10 of 16 watches in one month → (10/16)·12 = 7.5× the uniform baseline. + assert!( + (multiplier - 7.5).abs() < 1e-9, + "multiplier was {multiplier}" + ); + assert_eq!(count, 16); + + assert!( + !insights + .iter() + .any(|i| matches!(i, Insight::Seasonality { genre, .. } if genre == "Drama")), + "a spread-out genre must not yield a seasonality insight" + ); +} + +#[test] +fn seasonality_below_sample_threshold_is_absent() { + let rows: Vec = (0..14) + .map(|i| watch(&["Horror"], 4, date(2024, 10, 1 + i), 21)) + .collect(); + assert!(!kinds(&build_insights(&rows)).contains(&"seasonality")); +} + +// ── Medium timing ──────────────────────────────────────────────────────────── + +#[test] +fn medium_timing_detects_weekend_lift() { + let saturday = date(2024, 3, 9); // a Saturday + assert_eq!(saturday.weekday(), Weekday::Sat); + let friday = date(2024, 3, 8); + + let mut rows: Vec = Vec::new(); + // 6 cinema watches, 5 of them on Saturdays. + let cinema_days = [ + saturday, + saturday.with_day(16).unwrap(), + saturday.with_day(23).unwrap(), + saturday.with_day(30).unwrap(), + date(2024, 4, 6), // next month's first Saturday + friday, + ]; + for day in cinema_days { + let mut row = watch(&["Drama"], 4, day, 19); + row.watch_medium = Some("cinema".to_string()); + rows.push(row); + } + // 10 streaming watches on weekdays only (two Mon–Fri weeks) to keep the + // baseline weekend share driven by cinema alone. + for i in 0..10u32 { + let mut row = watch(&["Drama"], 4, friday, 21); + row.watch_medium = Some("streaming".to_string()); + row.watched_at = date(2024, 3, 4 + i % 5 + 7 * (i / 5)) + .and_hms_opt(21, 0, 0) + .unwrap(); + rows.push(row); + } + + let insights = build_insights(&rows); + let cinema = insights.iter().find_map(|i| match i { + Insight::MediumTiming { + medium, + multiplier, + sample_count, + .. + } if medium == "cinema" => Some((*multiplier, *sample_count)), + _ => None, + }); + let (multiplier, count) = cinema.expect("cinema timing expected"); + // Baseline weekend share = 5/16; cinema share = 5/6 → lift = (5/6)/(5/16) ≈ 2.67. + assert!( + (multiplier - 2.67).abs() < 0.01, + "multiplier was {multiplier}" + ); + assert_eq!(count, 6); +} + +#[test] +fn medium_timing_skipped_when_medium_mostly_missing() { + let mut rows: Vec = Vec::new(); + let first_saturday = date(2024, 3, 9); + for i in 0..6u32 { + let mut row = watch( + &["Drama"], + 4, + first_saturday + chrono::Duration::weeks(i64::from(i)), + 20, + ); + row.watch_medium = Some("cinema".to_string()); + rows.push(row); + } + for i in 0..10u32 { + let mut row = watch(&["Drama"], 4, date(2024, 3, 1 + i), 20); + row.watch_medium = None; // imports without a medium + rows.push(row); + } + + assert!(!kinds(&build_insights(&rows)).contains(&"medium_timing")); +} + +// ── Rhythm ─────────────────────────────────────────────────────────────────── + +#[test] +fn rhythm_finds_peak_window_and_days() { + // 12 watches at 22:30: five Tuesdays, four Wednesdays, three Thursdays. + let first_tuesday = date(2024, 3, 5); + assert_eq!(first_tuesday.weekday(), Weekday::Tue); + let mut rows: Vec = Vec::new(); + for day in 0..3u32 { + let count = 5 - day; // Tue ×5, Wed ×4, Thu ×3 + for week in 0..count { + rows.push(watch( + &["Drama"], + 4, + first_tuesday + + chrono::Duration::days(i64::from(day)) + + chrono::Duration::weeks(i64::from(week)), + 22, + )); + } + } + + let insights = build_insights(&rows); + let rhythm = insights.iter().find_map(|i| match i { + Insight::Rhythm { + peak_hour_start, + peak_hour_end, + top_days, + sample_count, + } => Some(( + *peak_hour_start, + *peak_hour_end, + top_days.clone(), + *sample_count, + )), + _ => None, + }); + let (start, end, days, count) = rhythm.expect("rhythm expected"); + // All watches sit at hour 22; the earliest 3-hour window that covers + // them starts at 20 (20, 21, 22). + assert_eq!(start, 20); + assert_eq!(end, 23); + // Every day carries ≥60% of the busiest day (5): Tue(5) Wed(4) Thu(3). + assert_eq!(days, vec![2, 3, 4], "Tue=2, Wed=3, Thu=4"); + assert_eq!(count, 12); +} + +#[test] +fn rhythm_skipped_when_timestamps_are_import_noise() { + let mut rows: Vec = Vec::new(); + for i in 0..12u32 { + // Exactly midnight — the signature of a CSV import with no time data. + let row = WatchRow { + movie_id: Uuid::new_v4(), + rating: 4, + watched_at: date(2024, 1, 1 + i).and_hms_opt(0, 0, 0).unwrap(), + watch_medium: None, + genres: vec!["Drama".to_string()], + }; + rows.push(row); + } + + assert!(!kinds(&build_insights(&rows)).contains(&"rhythm")); +} + +// ── Overall ────────────────────────────────────────────────────────────────── + +#[test] +fn empty_diary_yields_nothing() { + assert!(build_insights(&[]).is_empty()); +} + +#[test] +fn small_diary_yields_nothing() { + let rows: Vec = (0..4) + .map(|i| watch(&["Drama"], 4, date(2024, 1, 1 + i), 20)) + .collect(); + assert!(build_insights(&rows).is_empty()); +} + +#[test] +fn max_three_insights_per_category() { + // Six genres, all with a strong rating lift — only the three strongest survive. + let mut rows: Vec = Vec::new(); + let genres = [ + ("Thriller", 5u8), + ("Horror", 5), + ("Sci-Fi", 5), + ("Comedy", 5), + ("Romance", 5), + ("Documentary", 5), + ]; + for (genre, rating) in genres { + rows.extend((0..5).map(|i| watch(&[genre], rating, date(2024, 1, 1 + i), 20))); + } + // Baseline dramas rated 1 make every lift large. + rows.extend((0..10).map(|i| watch(&["Drama"], 1, date(2024, 3, 1 + i), 20))); + + let insights = build_insights(&rows); + let rating_lifts = insights + .iter() + .filter(|i| matches!(i, Insight::RatingLift { .. })) + .count(); + assert_eq!(rating_lifts, 3); +} diff --git a/crates/domain/src/testing/fakes.rs b/crates/domain/src/testing/fakes.rs index 74148b9e..d63e0cb6 100644 --- a/crates/domain/src/testing/fakes.rs +++ b/crates/domain/src/testing/fakes.rs @@ -103,9 +103,6 @@ impl FakeDiaryQuery { .insert(movie.id().value(), (movie, reviews)); } - /// Configures what `query_diary` returns. Without this, `query_diary` keeps - /// its original always-empty behavior — this is purely opt-in for tests that - /// need to prove entries actually flow through a use case. pub fn set_diary_page(&self, page: Paginated) { *self.diary_page.lock().unwrap() = Some(page); } diff --git a/crates/domain/src/testing/in_memory.rs b/crates/domain/src/testing/in_memory.rs index 0503889c..21a20c6d 100644 --- a/crates/domain/src/testing/in_memory.rs +++ b/crates/domain/src/testing/in_memory.rs @@ -347,9 +347,6 @@ impl WatchlistRepository for InMemoryWatchlistRepository { // ── InMemoryRemoteWatchlistRepository ───────────────────────────────────────── -/// Unlike `NoopRemoteWatchlistRepository`, this actually stores what it's given — -/// needed by tests that must prove a federated-watchlist read returns real seeded -/// data, not just an empty default that happens to match by coincidence. pub struct InMemoryRemoteWatchlistRepository { store: Mutex>, } @@ -405,10 +402,6 @@ impl RemoteWatchlistRepository for InMemoryRemoteWatchlistRepository { Ok(()) } - /// Real adapters derive `uuid` from the actor URL via a SQL-side hash — not - /// domain logic, so this fake doesn't replicate it. It simply returns - /// everything seeded, which is all callers in this workspace need: tests seed - /// exactly the entries for the owner under test. async fn get_by_derived_uuid( &self, _uuid: Uuid, diff --git a/crates/domain/src/testing/insights.rs b/crates/domain/src/testing/insights.rs new file mode 100644 index 00000000..2353d69a --- /dev/null +++ b/crates/domain/src/testing/insights.rs @@ -0,0 +1,52 @@ +use std::collections::HashMap; +use std::sync::{Arc, Mutex}; + +use async_trait::async_trait; + +use crate::{ + errors::DomainError, models::insights::WatchRow, ports::InsightsQuery, value_objects::UserId, +}; + +// ── PanicInsightsQuery ────────────────────────────────────────────────────── + +pub struct PanicInsightsQuery; + +#[async_trait] +impl InsightsQuery for PanicInsightsQuery { + async fn get_watch_rows(&self, _: &UserId) -> Result, DomainError> { + panic!("PanicInsightsQuery called") + } +} + +// ── InMemoryInsightsQuery ─────────────────────────────────────────────────── + +pub struct InMemoryInsightsQuery { + pub rows_by_user: Mutex>>, +} + +impl InMemoryInsightsQuery { + pub fn new() -> Arc { + Arc::new(Self { + rows_by_user: Mutex::new(HashMap::new()), + }) + } + + pub fn with_rows(user_id: uuid::Uuid, rows: Vec) -> Arc { + let query = Self::new(); + query.rows_by_user.lock().unwrap().insert(user_id, rows); + query + } +} + +#[async_trait] +impl InsightsQuery for InMemoryInsightsQuery { + async fn get_watch_rows(&self, user_id: &UserId) -> Result, DomainError> { + Ok(self + .rows_by_user + .lock() + .unwrap() + .get(&user_id.value()) + .cloned() + .unwrap_or_default()) + } +} diff --git a/crates/domain/src/testing/mod.rs b/crates/domain/src/testing/mod.rs index fb38fdfa..21299b3e 100644 --- a/crates/domain/src/testing/mod.rs +++ b/crates/domain/src/testing/mod.rs @@ -1,11 +1,13 @@ mod fakes; mod in_memory; +mod insights; mod noops; mod panics; mod wrapup; pub use fakes::*; pub use in_memory::*; +pub use insights::*; pub use noops::*; pub use panics::*; pub use wrapup::*; diff --git a/crates/domain/src/value_objects/ids.rs b/crates/domain/src/value_objects/ids.rs index f9878346..930a95d3 100644 --- a/crates/domain/src/value_objects/ids.rs +++ b/crates/domain/src/value_objects/ids.rs @@ -22,9 +22,6 @@ macro_rules! uuid_id { uuid_id!(MovieId); impl MovieId { - /// Derives a stable, deterministic UUID from an external metadata ID (e.g. `tmdb:12345`). - /// All instances that know a movie by the same external ID will produce the same MovieId, - /// enabling remote and local reviews to be linked to the same movie record. pub fn from_external(external_id: &crate::value_objects::ExternalMetadataId) -> Self { Self(Uuid::new_v5( &Uuid::NAMESPACE_URL, @@ -45,8 +42,6 @@ uuid_id!(GoalId); uuid_id!(PersonId); impl PersonId { - /// Deterministic UUIDv5 from an external person ID string. - /// "tmdb:12345" always maps to the same PersonId. pub fn from_external(external_id: &crate::models::person::ExternalPersonId) -> Self { Self(Uuid::new_v5( &Uuid::NAMESPACE_URL, diff --git a/crates/domain/src/value_objects/instance.rs b/crates/domain/src/value_objects/instance.rs index 7f3e13de..8bf92fe2 100644 --- a/crates/domain/src/value_objects/instance.rs +++ b/crates/domain/src/value_objects/instance.rs @@ -1,14 +1,11 @@ use super::{SocialIdentity, UserId}; -/// This instance's own identity on the network. Owns every URL and handle -/// derivation that used to be spelled with a bare `base_url: &str`. #[derive(Clone, Debug, PartialEq, Eq)] pub struct InstanceIdentity { base_url: String, } impl InstanceIdentity { - /// Normalizes away a trailing slash so every derived URL has exactly one. pub fn new(base_url: impl Into) -> Self { Self { base_url: base_url.into().trim_end_matches('/').to_string(), @@ -19,7 +16,6 @@ impl InstanceIdentity { &self.base_url } - /// Host and port, as it appears in a fediverse handle. pub fn host(&self) -> &str { self.base_url .split("://") @@ -47,7 +43,6 @@ impl InstanceIdentity { } } - /// Inverse of `actor_url_of`: decides whether an actor URL is ours. pub fn identify(&self, actor_url: &str) -> SocialIdentity { let prefix = format!("{}/users/", self.base_url); if let Some(uuid_str) = actor_url.strip_prefix(&prefix) diff --git a/crates/domain/src/value_objects/social.rs b/crates/domain/src/value_objects/social.rs index 9b6364cc..d51ad104 100644 --- a/crates/domain/src/value_objects/social.rs +++ b/crates/domain/src/value_objects/social.rs @@ -37,8 +37,6 @@ pub struct SocialActor { pub avatar_url: Option, } -/// Both directions of the follow edge between a viewer and a target. -/// `None` means no edge at all; `Some(Pending)` means requested but not accepted. #[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] pub struct FollowRelation { pub following: Option, diff --git a/crates/domain/src/value_objects/user.rs b/crates/domain/src/value_objects/user.rs index 6aa1a99c..f70b9c85 100644 --- a/crates/domain/src/value_objects/user.rs +++ b/crates/domain/src/value_objects/user.rs @@ -25,8 +25,6 @@ impl Username { const MIN_LENGTH: usize = 2; const MAX_LENGTH: usize = 30; - /// Accepts 2–30 chars: lowercase letters, digits, underscores, hyphens. - /// Lowercases input automatically. pub fn new(raw: String) -> Result { let s = raw.trim().to_lowercase(); if s.len() < Self::MIN_LENGTH || s.len() > Self::MAX_LENGTH { diff --git a/crates/presentation/src/csrf.rs b/crates/presentation/src/csrf.rs index 08b3c405..70464fd4 100644 --- a/crates/presentation/src/csrf.rs +++ b/crates/presentation/src/csrf.rs @@ -52,7 +52,6 @@ pub async fn csrf_middleware(mut req: Request, next: Next) -> Response { response } -/// Returns true if the form token does not match the cookie token. pub fn mismatch(token: &CsrfToken, form_value: &str) -> bool { token.0 != form_value || form_value.is_empty() } diff --git a/crates/presentation/src/handlers/insights.rs b/crates/presentation/src/handlers/insights.rs new file mode 100644 index 00000000..1403ba4f --- /dev/null +++ b/crates/presentation/src/handlers/insights.rs @@ -0,0 +1,81 @@ +use axum::{Json, extract::State}; + +use application::insights::get as get_insights_uc; +use domain::models::Insight; + +use crate::{errors::ApiError, extractors::AuthenticatedUser, state::AppState}; +use api_types::insights::{InsightDto, InsightsResponse}; + +fn to_dto(insight: Insight) -> InsightDto { + match insight { + Insight::RatingLift { + genre, + genre_avg, + user_avg, + delta, + sample_count, + } => InsightDto::RatingLift { + genre, + genre_avg, + user_avg, + delta, + sample_count, + }, + Insight::Seasonality { + genre, + month, + multiplier, + sample_count, + } => InsightDto::Seasonality { + genre, + month, + multiplier, + sample_count, + }, + Insight::MediumTiming { + medium, + multiplier, + weekend_share, + baseline_share, + sample_count, + } => InsightDto::MediumTiming { + medium, + multiplier, + weekend_share, + baseline_share, + sample_count, + }, + Insight::Rhythm { + peak_hour_start, + peak_hour_end, + top_days, + sample_count, + } => InsightDto::Rhythm { + peak_hour_start, + peak_hour_end, + top_days, + sample_count, + }, + } +} + +/// Live insights for the authenticated user's own diary. Private surface: +/// habits (late-night watching, mediums) are more sensitive than the taste +/// statistics already public on profiles (ADR-0010). +#[utoipa::path( + get, path = "/api/v1/insights", + responses( + (status = 200, body = InsightsResponse), + (status = 401, description = "Unauthorized"), + ), + security(("bearer_auth" = [])) +)] +pub async fn get_insights( + State(state): State, + user: AuthenticatedUser, +) -> Result, ApiError> { + let insights = get_insights_uc::execute(&state.app_ctx.deps.insights.get, user.0).await?; + Ok(Json(InsightsResponse { + insights: insights.into_iter().map(to_dto).collect(), + })) +} diff --git a/crates/presentation/src/handlers/mod.rs b/crates/presentation/src/handlers/mod.rs index 3681128d..0fc869cf 100644 --- a/crates/presentation/src/handlers/mod.rs +++ b/crates/presentation/src/handlers/mod.rs @@ -4,6 +4,7 @@ pub mod goals; mod helpers; pub mod images; pub mod import; +pub mod insights; pub mod integrations; pub mod movies; pub mod rss; diff --git a/crates/presentation/src/handlers/social.rs b/crates/presentation/src/handlers/social.rs index 459bdccc..3b257747 100644 --- a/crates/presentation/src/handlers/social.rs +++ b/crates/presentation/src/handlers/social.rs @@ -1104,9 +1104,6 @@ mod tests { } } - /// The bug: a local actor used to serialize as `local:{uuid}`, which - /// `identify` cannot parse, so every POST echoing this url back resolved - /// to Remote and was routed to ActivityPub. #[test] fn local_actor_dto_url_round_trips_back_to_the_same_identity() { let instance = InstanceIdentity::new("https://md.example"); diff --git a/crates/presentation/src/openapi/insights.rs b/crates/presentation/src/openapi/insights.rs new file mode 100644 index 00000000..b84e841f --- /dev/null +++ b/crates/presentation/src/openapi/insights.rs @@ -0,0 +1,8 @@ +use utoipa::OpenApi; + +#[derive(OpenApi)] +#[openapi( + paths(crate::handlers::insights::get_insights,), + components(schemas(api_types::insights::InsightsResponse, api_types::insights::InsightDto,)) +)] +pub struct InsightsDoc; diff --git a/crates/presentation/src/openapi/mod.rs b/crates/presentation/src/openapi/mod.rs index 08559ec9..79401054 100644 --- a/crates/presentation/src/openapi/mod.rs +++ b/crates/presentation/src/openapi/mod.rs @@ -2,6 +2,7 @@ mod auth; mod diary; mod goals; mod import; +mod insights; mod movies; mod search; mod social; @@ -46,6 +47,7 @@ fn build() -> utoipa::openapi::OpenApi { api.merge(goals::GoalsDoc::openapi()); api.merge(webhook::WebhookDoc::openapi()); api.merge(wrapup::WrapUpDoc::openapi()); + api.merge(insights::InsightsDoc::openapi()); #[cfg(feature = "federation")] api.merge(social::SocialDoc::openapi()); SecurityAddon.modify(&mut api); diff --git a/crates/presentation/src/routes.rs b/crates/presentation/src/routes.rs index efc883cc..53d7295f 100644 --- a/crates/presentation/src/routes.rs +++ b/crates/presentation/src/routes.rs @@ -460,6 +460,7 @@ fn api_routes(rate_limit: u64) -> Router { "/wrapups/{id}/report", routing::get(handlers::wrapup::get_report), ) + .route("/insights", routing::get(handlers::insights::get_insights)) .route( "/admin/reindex-search", routing::post(handlers::search::post_reindex_search), diff --git a/crates/presentation/src/tests/api_handlers.rs b/crates/presentation/src/tests/api_handlers.rs index 031d37bc..f79e7fe8 100644 --- a/crates/presentation/src/tests/api_handlers.rs +++ b/crates/presentation/src/tests/api_handlers.rs @@ -163,13 +163,6 @@ async fn person_credits_endpoint_returns_404_for_unknown_id() { assert_eq!(resp.status(), StatusCode::NOT_FOUND); } -/// Proves stub injection reaches the prebuilt deps, not just the `Repositories` -/// passed into `build_test_state_from`. Before the fix that added construct-time -/// stub injection, this failed on the assertion below — exactly the false-green -/// that would have silently broken the person-handler tests in Task 6. `AppContext` -/// no longer carries a `repos` field (Plan C2), so the original companion -/// assertion on `state.app_ctx.repos.person_query` was dropped along with the -/// field; the assertion that matters — that `deps` itself holds the stub — stays. #[test] fn injected_stub_reaches_prebuilt_deps() { let stub: Arc = Arc::new(PersonQueryStub); @@ -209,7 +202,6 @@ async fn get_watchlist_requires_auth() { // --- Pending-follower-count endpoint tests --- -/// Stub `FollowGraphQuery` returning a fixed pending-follower count. struct FixedPendingCount(usize); #[async_trait::async_trait] impl domain::ports::FollowGraphQuery for FixedPendingCount { @@ -264,10 +256,6 @@ impl domain::ports::FollowGraphQuery for FixedPendingCount { } } -/// The point of Task 4: one use case (`social::count_pending_followers`) now -/// serves both the classic UI (via `get_page_viewer`) and this new HTTP -/// endpoint. Seed the query port with a fixed count and assert the endpoint -/// reflects it verbatim. #[tokio::test] async fn pending_count_endpoint_reports_the_use_case_result() { let uid = domain::value_objects::UserId::from_uuid(Uuid::new_v4()); @@ -351,9 +339,6 @@ impl domain::ports::ApDocumentPort for FailingApDocument { } } -/// Swapping `ap_document` on the returned state is safe — unlike the `repos` -/// fields, handlers read `app_ctx.ap_document` directly and no prebuilt deps -/// struct carries a copy of it. #[cfg(feature = "federation")] fn state_with_failing_ap_document() -> crate::state::AppState { let mut state = test_app_state(); diff --git a/crates/presentation/src/tests/context.rs b/crates/presentation/src/tests/context.rs index c23a19bd..f555730e 100644 --- a/crates/presentation/src/tests/context.rs +++ b/crates/presentation/src/tests/context.rs @@ -1,12 +1,3 @@ -/// The prebuilt deps must point at the SAME port instances as the `Repositories` -/// they were built from. If build_deps is wired to a different Arc — or is called -/// before a repo is swapped — handlers in Plan B silently talk to the wrong -/// dependency, and no other test in Plan A would catch it. -/// -/// `AppContext` no longer carries a `repos` field (Plan C2 deleted it), so this -/// test builds its own `Repositories` locally, keeps a clone of the Arcs it -/// cares about, and hands the original to `build_test_state_from` — the same -/// construction `test_app_state()` uses internally. #[test] fn prebuilt_deps_share_port_instances_with_repos() { use std::sync::Arc; diff --git a/crates/presentation/src/tests/extractors.rs b/crates/presentation/src/tests/extractors.rs index a9652440..7ea73b46 100644 --- a/crates/presentation/src/tests/extractors.rs +++ b/crates/presentation/src/tests/extractors.rs @@ -16,12 +16,13 @@ use domain::{ Person, PersonCredits, PersonEnrichmentData, PersonId, Review, ReviewHistory, SearchQuery, SearchResults, UserStats, UserTrends, collections::{PageParams, Paginated}, + insights::WatchRow, }, ports::{ - AuthService, DiaryQuery, EventPublisher, FollowGraphQuery, MetadataClient, MovieCommand, - MovieQuery, ObjectStorage, PasswordHasher, PersonCommand, PersonQuery, PosterFetcherClient, - ReviewRepository, SearchCommand, SearchPort, StatsRepository, UserRepository, - WatchlistRepository, + AuthService, DiaryQuery, EventPublisher, FollowGraphQuery, InsightsQuery, MetadataClient, + MovieCommand, MovieQuery, ObjectStorage, PasswordHasher, PersonCommand, PersonQuery, + PosterFetcherClient, ReviewRepository, SearchCommand, SearchPort, StatsRepository, + UserRepository, WatchlistRepository, }, testing::InMemoryUserRepository, value_objects::{ @@ -169,6 +170,12 @@ impl StatsRepository for Panic { } } #[async_trait::async_trait] +impl InsightsQuery for Panic { + async fn get_watch_rows(&self, _: &UserId) -> Result, DomainError> { + panic!() + } +} +#[async_trait::async_trait] impl MetadataClient for Panic { async fn fetch_movie_metadata( &self, @@ -794,6 +801,7 @@ pub(crate) fn test_repositories() -> Repositories { federation_admin: Arc::new(domain::ports::noop::NoopFederationAdminQuery) as _, wrapup_stats: Arc::clone(&repo) as _, wrapup_repo: Arc::clone(&repo) as _, + insights: Arc::clone(&repo) as _, goal_command: Arc::clone(&repo) as _, goal_query: Arc::clone(&repo) as _, user_settings: Arc::clone(&repo) as _, diff --git a/crates/server/src/main.rs b/crates/server/src/main.rs index 63cd894f..a4f2eabd 100644 --- a/crates/server/src/main.rs +++ b/crates/server/src/main.rs @@ -218,6 +218,7 @@ async fn wire_dependencies() -> anyhow::Result<(AppState, axum::Router)> { federation_admin: Arc::new(domain::ports::noop::NoopFederationAdminQuery), wrapup_stats: db.wrapup_stats, wrapup_repo: db.wrapup_repo, + insights: db.insights, goal_command: db.goal_command, goal_query: db.goal_query, user_settings: db.user_settings, diff --git a/crates/server/tests/api_test.rs b/crates/server/tests/api_test.rs index c8ae1666..40ab7fbd 100644 --- a/crates/server/tests/api_test.rs +++ b/crates/server/tests/api_test.rs @@ -12,9 +12,8 @@ use domain::{ errors::DomainError, events::DomainEvent, models::{ - EntityType, ExternalPersonId, GeneratedToken, IndexableDocument, MetadataSearchCriteria, - Movie, Person, PersonCredits, PersonEnrichmentData, PersonId, SearchQuery, SearchResults, - User, + EntityType, ExternalPersonId, IndexableDocument, MetadataSearchCriteria, Movie, Person, + PersonCredits, PersonEnrichmentData, PersonId, SearchQuery, SearchResults, User, }, ports::{ AuthService, EventPublisher, MetadataClient, ObjectStorage, PasswordHasher, PersonCommand, @@ -27,11 +26,12 @@ use presentation::context::AppContext; use presentation::{routes, state::AppState}; use rss::RssAdapter; use sqlite::{ - SqliteDiaryRepository, SqliteMovieRepository, SqliteReviewRepository, SqliteStatsRepository, - migrate as sqlite_migrate, + SqliteDiaryRepository, SqliteInsightsQuery, SqliteMovieRepository, SqliteReviewRepository, + SqliteStatsRepository, migrate as sqlite_migrate, }; use sqlx::SqlitePool; use tower::ServiceExt; +use uuid::Uuid; struct NoopEventPublisher; #[async_trait] @@ -106,17 +106,6 @@ impl PasswordHasher for PanicHasher { } } -struct PanicAuth; -#[async_trait] -impl AuthService for PanicAuth { - async fn generate_token(&self, _: &UserId) -> Result { - panic!() - } - async fn validate_token(&self, _: &str) -> Result { - panic!() - } -} - struct NobodyUserRepo; #[async_trait] impl UserRepository for NobodyUserRepo { @@ -401,6 +390,10 @@ impl domain::ports::RemoteWatchlistRepository for PanicRemoteWatchlist { } } async fn test_app() -> Router { + test_app_with(domain::testing::FakeAuthService).await.0 +} + +async fn test_app_with(auth: impl AuthService + 'static) -> (Router, SqlitePool) { let pool = SqlitePool::connect("sqlite::memory:") .await .expect("in-memory SQLite failed"); @@ -432,6 +425,7 @@ async fn test_app() -> Router { federation_admin: Arc::new(domain::ports::noop::NoopFederationAdminQuery) as _, wrapup_stats: Arc::new(domain::testing::PanicWrapUpStatsQuery) as _, wrapup_repo: Arc::new(domain::testing::PanicWrapUpRepository) as _, + insights: Arc::new(SqliteInsightsQuery::new(pool.clone())) as _, goal_command: Arc::new(domain::testing::NoopGoalCommand), goal_query: Arc::new(domain::testing::NoopGoalQuery), user_settings: Arc::new(domain::testing::NoopUserSettingsRepository), @@ -441,7 +435,7 @@ async fn test_app() -> Router { }; let services = application::Services { - auth: Arc::new(PanicAuth), + auth: Arc::new(auth), password_hasher: Arc::new(PanicHasher), metadata: Arc::new(PanicMeta), poster_fetcher: Arc::new(PanicFetcher), @@ -485,7 +479,8 @@ async fn test_app() -> Router { rss_renderer: Arc::new(RssAdapter::new("http://localhost:3000".into())), }; - routes::build_router(state, axum::Router::new()) + let router = routes::build_router(state, axum::Router::new()); + (router, pool) } /// Inject a fake peer IP so the GovernorLayer can extract ConnectInfo. @@ -625,3 +620,127 @@ async fn get_movie_detail_html_returns_404_for_unknown_id() { assert_eq!(response.status(), StatusCode::NOT_FOUND); } + +// ── Insights ───────────────────────────────────────────────────────────────── + +async fn seed_insights_diary(pool: &SqlitePool, user_id: Uuid) { + // 12 thrillers rated 5 late at night, 12 comedies rated 2 — enough for a + // rating lift once genres come from movie_genres. + for i in 0..12u32 { + let movie_id = uuid::Uuid::new_v4().to_string(); + sqlx::query("INSERT INTO movies (id, title, release_year) VALUES (?, ?, ?)") + .bind(&movie_id) + .bind(format!("Thriller {i}")) + .bind(2020) + .execute(pool) + .await + .unwrap(); + sqlx::query("INSERT INTO movie_genres (movie_id, tmdb_id, name) VALUES (?, ?, ?)") + .bind(&movie_id) + .bind(44400 + i as i64) + .bind("Thriller") + .execute(pool) + .await + .unwrap(); + sqlx::query( + "INSERT INTO reviews (id, movie_id, user_id, rating, watched_at, created_at) \ + VALUES (?, ?, ?, ?, ?, ?)", + ) + .bind(uuid::Uuid::new_v4().to_string()) + .bind(&movie_id) + .bind(user_id.to_string()) + .bind(5) + .bind("2024-01-05 22:30:00") + .bind("2024-01-05 22:30:00") + .execute(pool) + .await + .unwrap(); + + let movie_id = uuid::Uuid::new_v4().to_string(); + sqlx::query("INSERT INTO movies (id, title, release_year) VALUES (?, ?, ?)") + .bind(&movie_id) + .bind(format!("Comedy {i}")) + .bind(2015) + .execute(pool) + .await + .unwrap(); + sqlx::query("INSERT INTO movie_genres (movie_id, tmdb_id, name) VALUES (?, ?, ?)") + .bind(&movie_id) + .bind(55300 + i as i64) + .bind("Comedy") + .execute(pool) + .await + .unwrap(); + sqlx::query( + "INSERT INTO reviews (id, movie_id, user_id, rating, watched_at, created_at) \ + VALUES (?, ?, ?, ?, ?, ?)", + ) + .bind(uuid::Uuid::new_v4().to_string()) + .bind(&movie_id) + .bind(user_id.to_string()) + .bind(2) + .bind("2024-03-10 13:00:00") + .bind("2024-03-10 13:00:00") + .execute(pool) + .await + .unwrap(); + } +} + +#[tokio::test] +async fn get_api_insights_returns_rating_lift_for_own_user() { + let (app, pool) = test_app_with(domain::testing::FakeAuthService).await; + let user_id = Uuid::new_v4(); + seed_insights_diary(&pool, user_id).await; + + let response = app + .oneshot(with_ip( + Request::builder() + .uri("/api/v1/insights") + .header("authorization", format!("Bearer {user_id}")) + .body(Body::empty()) + .unwrap(), + )) + .await + .unwrap(); + + let (status, bytes) = ( + response.status(), + response.into_body().collect().await.unwrap().to_bytes(), + ); + assert_eq!( + status, + StatusCode::OK, + "body: {}", + String::from_utf8(bytes.to_vec()).unwrap() + ); + let json: serde_json::Value = serde_json::from_slice(&bytes).unwrap(); + + let rating_lifts: Vec<&serde_json::Value> = json["insights"] + .as_array() + .unwrap() + .iter() + .filter(|i| i["kind"] == "rating_lift") + .collect(); + assert_eq!(rating_lifts.len(), 2, "thriller up, comedy down: {json}"); + + let thriller = rating_lifts + .iter() + .find(|i| i["genre"] == "Thriller") + .unwrap(); + assert_eq!(thriller["delta"], 1.5); + assert_eq!(thriller["sample_count"], 12); + // Timestamps are real → rhythm fires. Hours split evenly between 22 + // (thrillers) and 13 (comedies); every window covering either bucket ties, + // so the earliest one, 11:00–14:00, wins. Days tie too (Fri/Sun), sorted + // ascending. + let rhythm = json["insights"] + .as_array() + .unwrap() + .iter() + .find(|i| i["kind"] == "rhythm") + .expect("rhythm expected for real timestamps"); + assert_eq!(rhythm["peak_hour_start"], 11); + assert_eq!(rhythm["peak_hour_end"], 14); + assert_eq!(rhythm["top_days"], serde_json::json!([5, 7])); +} diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index e0d6b25b..59d9ec1b 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -375,8 +375,6 @@ pub fn parse_csv(content: &str) -> Vec { rows } -/// Returns a mutable reference to whichever text field currently has focus, -/// or `None` if the active widget is non-textual (e.g. a rating spinner). fn focused_input(app: &mut App) -> Option<&mut String> { match &mut app.screen { Screen::Setup(s) => Some(&mut s.api_url), diff --git a/crates/tui/src/main.rs b/crates/tui/src/main.rs index eef8cbc8..11f4f939 100644 --- a/crates/tui/src/main.rs +++ b/crates/tui/src/main.rs @@ -8,7 +8,6 @@ use tui::app::{self, Action, App, BulkImportStage, Command, Screen, SettingsFiel use tui::client::{ApiClient, ApiError}; use tui::config::Config; -/// Convert an API error into either `AuthExpired` (for 401s) or a `ShowError`. fn api_err_action(e: ApiError) -> Action { match e { ApiError::Unauthorized => Action::AuthExpired, @@ -27,7 +26,6 @@ fn main() -> anyhow::Result<()> { async fn run() -> anyhow::Result<()> { let mut config = Config::load(); - // env var override if let Ok(url) = std::env::var("MOVIES_API_URL") { match &mut config { Some(c) => c.api_url = url, @@ -48,7 +46,6 @@ async fn run() -> anyhow::Result<()> { let (tx, mut rx) = mpsc::channel::(64); let mut terminal = ratatui::init(); - // If we start directly in Main (saved token), trigger an initial diary load if matches!(app.screen, Screen::Main(_)) && let Some(token) = &saved_token { @@ -71,7 +68,6 @@ async fn run() -> anyhow::Result<()> { loop { terminal.draw(|f| tui::ui::render(f, &app))?; - // Poll keyboard — non-blocking with short timeout if event::poll(Duration::from_millis(50))? && let Event::Key(key) = event::read()? { @@ -89,7 +85,6 @@ async fn run() -> anyhow::Result<()> { } } - // Drain async results while let Ok(action) = rx.try_recv() { let cmds = app::update(&mut app, action); for cmd in cmds { @@ -273,7 +268,6 @@ fn tab_shortcut(code: KeyCode) -> Option { } fn key_to_action(app: &App, key: ratatui::crossterm::event::KeyEvent) -> Option { - // Ctrl+C always quits if key.modifiers.contains(KeyModifiers::CONTROL) && key.code == KeyCode::Char('c') { return Some(Action::Quit); } diff --git a/crates/tui/src/tests/app.rs b/crates/tui/src/tests/app.rs index cde757c8..550d928f 100644 --- a/crates/tui/src/tests/app.rs +++ b/crates/tui/src/tests/app.rs @@ -501,9 +501,6 @@ fn auth_ok_uses_app_api_url_for_main_state() { // ── parse_csv ───────────────────────────────────────────────────────────── -// CSV column order matches the export format: -// title,year,director,rating,comment,watched_at,external_metadata_id - #[test] fn parse_csv_valid_row_with_title() { let csv = "title,year,director,rating,comment,watched_at,external_metadata_id\nThe Matrix,1999,Wachowski,5,,1999-03-31T00:00:00,\n"; diff --git a/docs/adr/0010-conservative-live-insights.md b/docs/adr/0010-conservative-live-insights.md new file mode 100644 index 00000000..38983505 --- /dev/null +++ b/docs/adr/0010-conservative-live-insights.md @@ -0,0 +1,62 @@ +# Insights are live, conservative, and computed by a pure analyzer over denormalized rows + +Insights (a new always-on surface, distinct from WrapUp and from plain +`UserTrends`) analyse a user's diary for behavioural patterns — rating lift by +genre, genre seasonality, medium timing, watch-time rhythm. Every insight +compares the user against their own baseline rather than reporting a ranking. + +## Decision + +- **Live, not precomputed.** Unlike WrapUp (async worker, stored + `report_json`), Insights are computed on every request. The underlying query + is one join over `reviews` + `movies` + genus tables, the same cost class as + existing `StatsRepository` aggregates; a materialization layer is only earned + if diaries grow provably too large for that. +- **Honesty by suppression, not disclaimers.** Insights are conservative: any + candidate below sample-size minimums (≥3–5 ratings per genre for rating + lift, ≥15–20 rows for seasonality, ≥5 rows per medium bucket) or strength + thresholds (+0.3 stars, 1.5×) is silently not shown, and whole categories + collapse when their inputs are unreliable (>~30% `NULL` `watch_medium`; + >50% of rows at exactly `00:00`, which betrays imported CSVs that carry no + real time). An empty-insights page is the correct rendering of a small or + noisy diary — the UI shows an empty state, not filler. This is deliberate; + do not "fix" it by lowering thresholds or adding hedges. +- **Fetch rows, then analyze in the domain.** Rejected the `StatsRepository` + pattern (aggregation in adapter SQL) as the primary carrier of insight logic: + thresholds and baseline comparisons would hide inside SQL, untestable and + duplicated per database. Instead the port + `InsightsQuery::get_watch_rows() -> Vec` fetches one denormalized + row per review (genres, medium, watched time), analogous to + `WrapUpStatsQuery::get_reviews_with_profiles` but without WrapUp's report-era + fields. A pure analyzer `insights_analyzer::build_insights(rows) -> Vec` + owns every threshold and comparison, unit-tested without a database. Rewatch + detection also lives in the analyzer — grouping rows by movie and keeping the + earliest, exactly like `wrapup_analyzer::compute_rewatch_stats` — so there is + one rewatch definition per analyzer and the adapters stay shapeless + "rows in, rows out". **Rating lift uses first-time entries only** (rewatches + inflate genre ratings by choice, not by property), while seasonality and + timing count every entry, rewatch or not. +- **Multi-genre rows are counted per genre, unnormalized.** A review of a + movie with three genres contributes to all three genre distributions. Genre + insights are frequency comparisons, not probabilities over reviews. +- **Private surface for now.** Insights render only on the user's own page + (`/insights`, own data). They expose habits (late-night watching, mediums), + which are more sensitive than the taste statistics already public on + profiles; sharing is an opt-in feature to design later, not a default. + +## Considered Options + +- **SQL aggregation in adapters (stats pattern)** — rejected for the primary + logic path: threshold rules would fragment into per-database SQL and resist + unit testing; SQL still wins where it is just "rows in, rows out". +- **Insights as WrapUp sections** — rejected: WrapUp is a dated snapshot, + insights decay/shift with every logged review and belong to a live surface; + whether to feed selected insights into future WrapUps is a later product + decision. +- **Rewatch detection in adapter SQL** — rejected: it would fork the rewatch + definition already applied by `wrapup_analyzer::compute_rewatch_stats`; the + analyzer groups by movie and keeps the earliest review, same as WrapUp. +- **Generic `Insight { title, description }` text payload** — rejected: typed + serde-tagged variants (`RatingLift`, `Seasonality`, `MediumTiming`, `Rhythm`, + each carrying `sample_count`) let the SPA render data-backed cards, show + "why nothing here yet", and stay stable across copy changes. diff --git a/spa/bun.lock b/spa/bun.lock index 793c490c..9e93bb99 100644 --- a/spa/bun.lock +++ b/spa/bun.lock @@ -1,5 +1,6 @@ { "lockfileVersion": 1, + "configVersion": 0, "workspaces": { "": { "name": "vite-app", @@ -18,6 +19,7 @@ "cmdk": "^1.1.1", "date-fns": "^4.4.0", "embla-carousel-react": "^8.6.0", + "html2canvas-pro": "^2.0.4", "i18next": "^26.3.1", "input-otp": "^1.4.2", "lucide-react": "^1.17.0", @@ -844,6 +846,8 @@ "html-parse-stringify": ["html-parse-stringify@3.0.1", "", { "dependencies": { "void-elements": "3.1.0" } }, "sha512-KknJ50kTInJ7qIScF3jeaFRpMpE8/lfiTdzf/twXyPBLAGrLRTmkz3AdTnKeh40X8k9L2fdYwEp/42WGXIRGcg=="], + "html2canvas-pro": ["html2canvas-pro@2.5.2", "", {}, "sha512-uka+0oVG/yB4B7U9M/+aJFF0aKZVmMWZKi4unemkQh537cLu3jXPvZtXakTuPl3taOI+O43RhuV6kn2igk1Z6g=="], + "http-errors": ["http-errors@2.0.1", "", { "dependencies": { "depd": "~2.0.0", "inherits": "~2.0.4", "setprototypeof": "~1.2.0", "statuses": "~2.0.2", "toidentifier": "~1.0.1" } }, "sha512-4FbRdAX+bSdmo4AUFuS0WNiPz8NgFt+r8ThgNWmlrjQjt1Q7ZR9+zTlce2859x4KSXrwIsaeTqDoKQmtP8pLmQ=="], "https-proxy-agent": ["https-proxy-agent@5.0.1", "", { "dependencies": { "agent-base": "6", "debug": "4" } }, "sha512-dFcAjpTQFgoLMzC2VwU+C/CbS7uRL0lWmxDITmqm7C+7F0Odmj6s9l6alZc6AELXhrnggM2CeWSXHGOdX2YtwA=="], diff --git a/spa/src/features/insights.ts b/spa/src/features/insights.ts new file mode 100644 index 00000000..66d25bab --- /dev/null +++ b/spa/src/features/insights.ts @@ -0,0 +1,61 @@ +import { useQuery } from "@tanstack/react-query" +import { get } from "@/lib/api/client" + +export type RatingLiftInsight = { + kind: "rating_lift" + genre: string + genre_avg: number + user_avg: number + delta: number + sample_count: number +} + +export type SeasonalityInsight = { + kind: "seasonality" + genre: string + month: number + multiplier: number + sample_count: number +} + +export type MediumTimingInsight = { + kind: "medium_timing" + medium: string + multiplier: number + weekend_share: number + baseline_share: number + sample_count: number +} + +export type RhythmInsight = { + kind: "rhythm" + peak_hour_start: number + peak_hour_end: number + top_days: number[] + sample_count: number +} + +export type Insight = + | RatingLiftInsight + | SeasonalityInsight + | MediumTimingInsight + | RhythmInsight + +export type InsightsResponse = { + insights: Insight[] +} + +export const insightsKeys = { + all: ["insights"] as const, +} + +function getInsights() { + return get("/insights") +} + +export function useInsights() { + return useQuery({ + queryKey: insightsKeys.all, + queryFn: getInsights, + }) +} diff --git a/spa/src/locales/en.json b/spa/src/locales/en.json index 2517a775..f187cfce 100644 --- a/spa/src/locales/en.json +++ b/spa/src/locales/en.json @@ -258,6 +258,25 @@ "plexStep2": "Add the URL above, replacing YOUR_TOKEN with your generated token", "plexStep3": "Plex sends scrobble events when a movie is watched to 90%+ (requires Plex Pass)" }, + "insights": { + "title": "Insights", + "subtitle": "Patterns your diary reveals", + "emptyTitle": "No insights yet", + "emptyDesc": "Insights appear once there is enough data to say something honest — keep logging films.", + "taste": "Your taste", + "seasons": "Seasons", + "mediums": "How you watch", + "rhythm": "When you watch", + "ratingLiftUp": "You rate {{genre}} higher than your own average — {{genreAvg}}★ vs {{userAvg}}★", + "ratingLiftDown": "You rate {{genre}} lower than your own average — {{genreAvg}}★ vs {{userAvg}}★", + "yourAverage": "your average", + "seasonality": "{{genre}} is your {{month}} genre — {{multiplier}}× your yearly baseline", + "mediumTiming": "You watch {{medium}} on weekends {{multiplier}}× more than your baseline", + "weekends": "weekends", + "yourBaseline": "your baseline", + "rhythm": "You watch mostly {{start}}–{{end}} on {{days}}", + "filmsSample": "{{count}} films" + }, "wrapup": { "title": "Year Wrap-Up", "noWrapUps": "No wrap-ups", diff --git a/spa/src/routeTree.gen.ts b/spa/src/routeTree.gen.ts index 866d4df9..7dbb8bc6 100644 --- a/spa/src/routeTree.gen.ts +++ b/spa/src/routeTree.gen.ts @@ -16,6 +16,7 @@ import { Route as AppIndexRouteImport } from './routes/_app/index' import { Route as AppSocialRouteImport } from './routes/_app/social' import { Route as AppSearchRouteImport } from './routes/_app/search' import { Route as AppProfileRouteImport } from './routes/_app/profile' +import { Route as AppInsightsRouteImport } from './routes/_app/insights' import { Route as AppDiaryRouteImport } from './routes/_app/diary' import { Route as AppSettingsIndexRouteImport } from './routes/_app/settings/index' import { Route as AppWrapupIdRouteImport } from './routes/_app/wrapup.$id' @@ -63,6 +64,11 @@ const AppProfileRoute = AppProfileRouteImport.update({ path: '/profile', getParentRoute: () => AppRoute, } as any) +const AppInsightsRoute = AppInsightsRouteImport.update({ + id: '/insights', + path: '/insights', + getParentRoute: () => AppRoute, +} as any) const AppDiaryRoute = AppDiaryRouteImport.update({ id: '/diary', path: '/diary', @@ -129,6 +135,7 @@ export interface FileRoutesByFullPath { '/login': typeof LoginRoute '/register': typeof RegisterRoute '/diary': typeof AppDiaryRoute + '/insights': typeof AppInsightsRoute '/profile': typeof AppProfileRoute '/search': typeof AppSearchRoute '/social': typeof AppSocialRoute @@ -148,6 +155,7 @@ export interface FileRoutesByTo { '/login': typeof LoginRoute '/register': typeof RegisterRoute '/diary': typeof AppDiaryRoute + '/insights': typeof AppInsightsRoute '/profile': typeof AppProfileRoute '/search': typeof AppSearchRoute '/social': typeof AppSocialRoute @@ -170,6 +178,7 @@ export interface FileRoutesById { '/login': typeof LoginRoute '/register': typeof RegisterRoute '/_app/diary': typeof AppDiaryRoute + '/_app/insights': typeof AppInsightsRoute '/_app/profile': typeof AppProfileRoute '/_app/search': typeof AppSearchRoute '/_app/social': typeof AppSocialRoute @@ -193,6 +202,7 @@ export interface FileRouteTypes { | '/login' | '/register' | '/diary' + | '/insights' | '/profile' | '/search' | '/social' @@ -212,6 +222,7 @@ export interface FileRouteTypes { | '/login' | '/register' | '/diary' + | '/insights' | '/profile' | '/search' | '/social' @@ -233,6 +244,7 @@ export interface FileRouteTypes { | '/login' | '/register' | '/_app/diary' + | '/_app/insights' | '/_app/profile' | '/_app/search' | '/_app/social' @@ -307,6 +319,13 @@ declare module '@tanstack/react-router' { preLoaderRoute: typeof AppProfileRouteImport parentRoute: typeof AppRoute } + '/_app/insights': { + id: '/_app/insights' + path: '/insights' + fullPath: '/insights' + preLoaderRoute: typeof AppInsightsRouteImport + parentRoute: typeof AppRoute + } '/_app/diary': { id: '/_app/diary' path: '/diary' @@ -396,6 +415,7 @@ declare module '@tanstack/react-router' { interface AppRouteChildren { AppDiaryRoute: typeof AppDiaryRoute + AppInsightsRoute: typeof AppInsightsRoute AppProfileRoute: typeof AppProfileRoute AppSearchRoute: typeof AppSearchRoute AppSocialRoute: typeof AppSocialRoute @@ -415,6 +435,7 @@ interface AppRouteChildren { const AppRouteChildren: AppRouteChildren = { AppDiaryRoute: AppDiaryRoute, + AppInsightsRoute: AppInsightsRoute, AppProfileRoute: AppProfileRoute, AppSearchRoute: AppSearchRoute, AppSocialRoute: AppSocialRoute, diff --git a/spa/src/routes/_app/insights.tsx b/spa/src/routes/_app/insights.tsx new file mode 100644 index 00000000..06ada321 --- /dev/null +++ b/spa/src/routes/_app/insights.tsx @@ -0,0 +1,263 @@ +import { createFileRoute } from "@tanstack/react-router" +import type { ReactNode } from "react" +import { useTranslation } from "react-i18next" +import { + CalendarDays, + Clapperboard, + Clock, + Lightbulb, + Sparkles, + TrendingDown, + TrendingUp, +} from "lucide-react" +import { BackButton } from "@/components/back-button" +import { Badge } from "@/components/ui/badge" +import { Card, CardContent, CardHeader, CardTitle } from "@/components/ui/card" +import { EmptyState } from "@/components/empty-state" +import { Skeleton } from "@/components/ui/skeleton" +import { WATCH_MEDIUMS } from "@/lib/watch-mediums" +import { useInsights, type Insight } from "@/features/insights" +import { useDocumentTitle } from "@/hooks/use-document-title" + +export const Route = createFileRoute("/_app/insights")({ + component: InsightsPage, +}) + +const MONTH_BASE = new Date(2024, 0, 1) // any year; only the month name matters +const MONDAY_BASE = new Date(2024, 2, 4) // 2024-03-04 is a Monday + +function isKind(kind: K) { + return (insight: Insight): insight is Extract => insight.kind === kind +} + +function monthName(month: number): string { + return new Intl.DateTimeFormat(undefined, { month: "long" }).format( + new Date(MONTH_BASE.getFullYear(), month - 1, 1), + ) +} + +function dayNames(days: number[]): string[] { + const fmt = new Intl.DateTimeFormat(undefined, { weekday: "short" }) + return days.map((d) => fmt.format(new Date(MONDAY_BASE.getFullYear(), MONDAY_BASE.getMonth(), MONDAY_BASE.getDate() + d - 1))) +} + +function mediumLabel(medium: string, t: (key: string) => string): string { + const def = WATCH_MEDIUMS.find((m) => m.value === medium) + return def ? t(def.labelKey) : medium +} + +function hourLabel(hour: number): string { + return `${String(hour).padStart(2, "0")}:00` +} + +function InsightsPage() { + const { t } = useTranslation() + const { data, isPending } = useInsights() + + useDocumentTitle(t("insights.title")) + if (isPending) return + + const insights = data?.insights ?? [] + + return ( +
+ +
+

+ + {t("insights.title")} +

+

{t("insights.subtitle")}

+
+ + {insights.length === 0 ? ( + + ) : ( + <> + } + /> + } + /> + } + /> + } + /> + + )} +
+ ) +} + +function InsightGroup({ + title, + items, + render, +}: { + title: string + items: T[] + render: (insight: T) => ReactNode +}) { + if (items.length === 0) return null + return ( +
+

{title}

+ {items.map((insight, i) => ( +
{render(insight)}
+ ))} +
+ ) +} + +type RatingLiftCardProps = { insight: Extract } +type SeasonalityCardProps = { insight: Extract } +type MediumTimingCardProps = { insight: Extract } +type RhythmCardProps = { insight: Extract } + +function RatingLiftCard({ insight }: RatingLiftCardProps) { + const { t } = useTranslation() + const up = insight.delta > 0 + const Trend = up ? TrendingUp : TrendingDown + return ( + + + + + {t(up ? "insights.ratingLiftUp" : "insights.ratingLiftDown", { + genre: insight.genre, + genreAvg: insight.genre_avg.toFixed(1), + userAvg: insight.user_avg.toFixed(1), + })} + + + +
+ + {insight.genre_avg.toFixed(1)}★ · {insight.genre} + + + {t("insights.yourAverage")}: {insight.user_avg.toFixed(1)}★ + + +
+ + + ) +} + +function SeasonalityCard({ insight }: SeasonalityCardProps) { + const { t } = useTranslation() + return ( + + + + + {t("insights.seasonality", { + genre: insight.genre, + multiplier: insight.multiplier.toFixed(1), + month: monthName(insight.month), + })} + + + +
+ + {insight.multiplier.toFixed(1)}× · {monthName(insight.month)} + + {insight.genre} + +
+ + + ) +} + +function MediumTimingCard({ insight }: MediumTimingCardProps) { + const { t } = useTranslation() + return ( + + + + + {t("insights.mediumTiming", { + medium: mediumLabel(insight.medium, t), + multiplier: insight.multiplier.toFixed(1), + })} + + + +
+ + {Math.round(insight.weekend_share * 100)}% · {t("insights.weekends")} + + + {t("insights.yourBaseline")}: {Math.round(insight.baseline_share * 100)}% + + +
+ + + ) +} + +function RhythmCard({ insight }: RhythmCardProps) { + const { t } = useTranslation() + return ( + + + + + {t("insights.rhythm", { + start: hourLabel(insight.peak_hour_start), + end: hourLabel(insight.peak_hour_end), + days: dayNames(insight.top_days).join(", "), + })} + + + +
+ + {hourLabel(insight.peak_hour_start)}–{hourLabel(insight.peak_hour_end)} + + {dayNames(insight.top_days).join(", ")} + +
+ + + ) +} + +function SampleBadge({ count }: { count: number }) { + const { t } = useTranslation() + return {t("insights.filmsSample", { count })} +} + +function InsightsSkeleton() { + return ( +
+ + + +
+ {[0, 1, 2].map((i) => ( + + ))} +
+
+ ) +} diff --git a/spa/src/routes/_app/profile.tsx b/spa/src/routes/_app/profile.tsx index 6947d983..5856ddf8 100644 --- a/spa/src/routes/_app/profile.tsx +++ b/spa/src/routes/_app/profile.tsx @@ -1,7 +1,7 @@ import { createFileRoute, Link } from "@tanstack/react-router" import { useState } from "react" import { useTranslation } from "react-i18next" -import { ChevronDown, ChevronRight, Plus, Settings, Sparkles } from "lucide-react" +import { ChevronDown, ChevronRight, Lightbulb, Plus, Settings, Sparkles } from "lucide-react" import { Button } from "@/components/ui/button" import { ProfileView, ProfileSkeleton } from "@/components/profile-view" import { useAuth } from "@/components/auth-provider" @@ -51,6 +51,15 @@ function ProfilePage() { actions={ <> + + + + + ) : insights.length === 0 ? ( {t("insights.yourAverage")}: {insight.user_avg.toFixed(1)}★ - + @@ -193,7 +202,7 @@ function SeasonalityCard({ insight }: SeasonalityCardProps) { {insight.multiplier.toFixed(1)}× · {monthName(insight.month)} {insight.genre} - + @@ -227,7 +236,7 @@ function MediumTimingCard({ insight }: MediumTimingCardProps) { {t("insights.yourBaseline")}: {Math.round(insight.baseline_share * 100)}% - + @@ -259,16 +268,19 @@ function RhythmCard({ insight }: RhythmCardProps) { {hourLabel(insight.peak_hour_start)}–{hourLabel(insight.peak_hour_end)} {dayNames(insight.top_days).join(", ")} - + ) } -function SampleBadge({ count }: { count: number }) { +// Rating lift counts distinct films (first watches only); every other +// category counts diary entries, rewatches included. +function SampleBadge({ unit, count }: { unit: "films" | "watches"; count: number }) { const { t } = useTranslation() - return {t("insights.filmsSample", { count })} + const key = unit === "films" ? "insights.filmsSample" : "insights.watchesSample" + return {t(key, { count })} } function InsightsSkeleton() { -- 2.49.1