feat(db): add series, season and episode model #71

naps62-yolo left a comment

Reviewed 870f9d20e498573aff3fe160316020e3c75b44dd. Findings on the diff below, plus one that has no line to sit on:

0003 added movie_releases so a cached release can be associated with a title. There is no episode counterpart, so §9.3 manual search has nowhere to record an episode/release association. Out of scope for #34 — worth its own issue rather than widening this one.

Reviewed `870f9d20e498573aff3fe160316020e3c75b44dd`. Findings on the diff below, plus one that has no line to sit on: 0003 added `movie_releases` so a cached release can be associated with a title. There is no episode counterpart, so §9.3 manual search has nowhere to record an episode/release association. Out of scope for #34 — worth its own issue rather than widening this one.

crates/arr-core/src/lib.rs Outdated

						
				@@ -184,2 +187,4 @@

				}

				#[derive(Clone, Debug, Default, Eq, PartialEq)]

				pub struct SeriesOverrides {

naps62-yolo commented

SeriesOverrides is field-for-field identical to MovieOverrides, but policy::evaluate takes &MovieOverrides (policy.rs:58, policy.rs:106). As written, a series cannot be evaluated against a policy without a conversion that does not exist yet, and the two structs will drift.

DESIGN §5.1 describes one override mechanism, not one per aggregate. Reuse a single type — rename MovieOverrides to TitleOverrides and drop this one.

`SeriesOverrides` is field-for-field identical to `MovieOverrides`, but `policy::evaluate` takes `&MovieOverrides` (`policy.rs:58`, `policy.rs:106`). As written, a series cannot be evaluated against a policy without a conversion that does not exist yet, and the two structs will drift. DESIGN §5.1 describes one override mechanism, not one per aggregate. Reuse a single type — rename `MovieOverrides` to `TitleOverrides` and drop this one.

naps62-yolo marked this conversation as resolved

crates/arr-core/src/lib.rs

						
				@@ -186,0 +214,4 @@

				}

				#[derive(Clone, Copy, Debug, Eq, PartialEq)]

				pub enum EpisodeState {

naps62-yolo commented

EpisodeState is Missing | Downloading | Available; the column added in this migration is CHECK (state IN ('missing', 'grabbed', 'imported')).

Movies already have this split, and 0001_movies.sql says arr-core "owns the canonical enum this mirrors" — it does not mirror it. This PR makes it two mismatches instead of one. Pick one vocabulary here, or leave a comment saying which side is authoritative.

`EpisodeState` is `Missing | Downloading | Available`; the column added in this migration is `CHECK (state IN ('missing', 'grabbed', 'imported'))`. Movies already have this split, and `0001_movies.sql` says arr-core "owns the canonical enum this mirrors" — it does not mirror it. This PR makes it two mismatches instead of one. Pick one vocabulary here, or leave a comment saying which side is authoritative.

naps62-yolo marked this conversation as resolved

crates/arr-db/migrations/0005_series.sql Outdated

						
				@@ -0,0 +1,97 @@

				CREATE TABLE series (

naps62-yolo commented

series stores nothing about upstream status, so DESIGN §4.2 ended — "series finished upstream and complete" — cannot be derived from this schema. The other four statuses come from episode rows; this one needs TMDB's series status persisted.

§4's sketch omits it, §4.2 requires it. Per CLAUDE.md the design document wins, so either add the column or record on #34 that ended is unimplementable until it exists.

`series` stores nothing about upstream status, so DESIGN §4.2 `ended` — "series finished upstream and complete" — cannot be derived from this schema. The other four statuses come from episode rows; this one needs TMDB's series status persisted. §4's sketch omits it, §4.2 requires it. Per CLAUDE.md the design document wins, so either add the column or record on #34 that `ended` is unimplementable until it exists.

naps62-yolo marked this conversation as resolved

crates/arr-db/migrations/0005_series.sql Outdated

						
				@@ -0,0 +14,4 @@

				CREATE INDEX series_root ON series (root_id);

				CREATE TRIGGER movies_require_movie_root

naps62-yolo commented

Two things about this block:

movies_require_movie_root and its update twin change the movies invariant, in a migration named for series. Nothing in the file says why they arrive here (they do belong to this change — TV roots only start existing now — but a reader of 0005_series.sql has no way to know that).
The file carries no comments at all, while 0001–0004 cite DESIGN sections throughout. That convention is what makes the schema readable without the design document open.

Two things about this block: - `movies_require_movie_root` and its update twin change the movies invariant, in a migration named for series. Nothing in the file says why they arrive here (they do belong to this change — TV roots only start existing now — but a reader of `0005_series.sql` has no way to know that). - The file carries no comments at all, while 0001–0004 cite DESIGN sections throughout. That convention is what makes the schema readable without the design document open.

naps62-yolo marked this conversation as resolved

						
				@@ -0,0 +74,4 @@

				CREATE INDEX episodes_state ON episodes (state);

				INSERT INTO policies (

naps62-yolo commented

The TV policies are seeded by copying rows matched on name = 'Movies — main' / 'Movies — kids', and the roots below are then matched on name = 'TV — main' / 'TV — kids'.

If a later migration renames or removes the 0002 seed rows, both inserts match zero rows and this migration succeeds having created no TV policies and no TV roots — a silent seed failure on any database migrating through it. Spell the policy values out as 0002 does, or guard with RAISE(ABORT, ...) when the source row is absent.

The TV policies are seeded by copying rows matched on `name = 'Movies — main'` / `'Movies — kids'`, and the roots below are then matched on `name = 'TV — main'` / `'TV — kids'`. If a later migration renames or removes the 0002 seed rows, both inserts match zero rows and this migration succeeds having created no TV policies and no TV roots — a silent seed failure on any database migrating through it. Spell the policy values out as 0002 does, or guard with `RAISE(ABORT, ...)` when the source row is absent.

naps62-yolo marked this conversation as resolved

						
				@@ -0,0 +76,4 @@

				INSERT INTO policies (

				    name, required_audio, dub_blacklist, hdr_rules, size_bands,

				    resolution_pref, source_weights, score_weights

naps62-yolo commented

The TV policies inherit source_weights from the movie policies: {"Remux":4,"BluRay":3,"WEB-DL":2,...} for main, BluRay-biased.

TV releases are overwhelmingly WEB-DL, so the tiebreaker points the wrong way out of the box. 0002 set these per root deliberately (the kids policy biases WEB-DL for pt-PT dubs); copying them wholesale drops that reasoning.

The TV policies inherit `source_weights` from the movie policies: `{"Remux":4,"BluRay":3,"WEB-DL":2,...}` for main, BluRay-biased. TV releases are overwhelmingly WEB-DL, so the tiebreaker points the wrong way out of the box. 0002 set these per root deliberately (the kids policy biases WEB-DL for pt-PT dubs); copying them wholesale drops that reasoning.

naps62-yolo marked this conversation as resolved

crates/arr-db/src/lib.rs

						
				@@ -250,0 +302,4 @@

				            .fetch_one(db.pool())

				            .await

				            .expect("movie schema remains available");

				        assert_eq!(movie_count, 0);

naps62-yolo commented

2026-08-22 21:41:51 +01:00

Counting rows in an untouched movies table asserts nothing — it is 0 whether or not this migration broke the movie schema. Either drop it or assert something the migration could plausibly break, e.g. that a movie insert against a movie root still succeeds.

Counting rows in an untouched `movies` table asserts nothing — it is 0 whether or not this migration broke the movie schema. Either drop it or assert something the migration could plausibly break, e.g. that a movie insert against a movie root still succeeds.

naps62-yolo marked this conversation as resolved

naps62-yolo added 1 commit 2026-08-22 21:41:38 +01:00

fix(db): address TV model review

ci / rust (pull_request) Successful in 1m49s

Details

ci / web (pull_request) Successful in 34s

Details

b4c6c872bb

naps62-yolo reviewed

2026-08-22 21:41:51 +01:00

naps62-yolo commented

Kept the main TV source order because DESIGN §5.5 explicitly defines Remux > BluRay > WEB-DL as the source tiebreaker. The TV kids policy still uses the §5.2 WEB-DL bias for pt-PT dubbing.

Kept the main TV source order because DESIGN §5.5 explicitly defines `Remux > BluRay > WEB-DL` as the source tiebreaker. The TV kids policy still uses the §5.2 WEB-DL bias for pt-PT dubbing.

naps62-yolo commented

2026-08-22 21:42:02 +01:00

Addressed the review in b4c6c87: shared title overrides, canonical episode states, persisted upstream completion, explicit TV policy seeds, migration rationale, and a real movie-schema compatibility assertion.

Kept the main TV source order because DESIGN §5.5 explicitly defines Remux > BluRay > WEB-DL; the TV kids policy retains the §5.2 WEB-DL bias for pt-PT dubbing.

Addressed the review in b4c6c87: shared title overrides, canonical episode states, persisted upstream completion, explicit TV policy seeds, migration rationale, and a real movie-schema compatibility assertion. Kept the main TV source order because DESIGN §5.5 explicitly defines `Remux > BluRay > WEB-DL`; the TV kids policy retains the §5.2 WEB-DL bias for pt-PT dubbing.

naps62-yolo reviewed

naps62-yolo left a comment

Reviewed b4c6c872bbae061a1eb61591044adc1da556b14e. Previous findings addressed — explicit policy seeds, upstream_ended, TitleOverrides, migration comments, and a test that actually writes a movie row. Three follow-ups on the new commit, all on the diff below.

Reviewed `b4c6c872bbae061a1eb61591044adc1da556b14e`. Previous findings addressed — explicit policy seeds, `upstream_ended`, `TitleOverrides`, migration comments, and a test that actually writes a movie row. Three follow-ups on the new commit, all on the diff below.

crates/arr-core/src/lib.rs

						
				@@ -186,0 +211,4 @@

				}

				#[derive(Clone, Copy, Debug, Eq, PartialEq)]

				pub enum EpisodeState {

naps62-yolo commented

MovieState and EpisodeState are now the same two-line enum twice over, which is the duplication TitleOverrides just removed one instance of. Worth the same treatment, or a comment saying the two are expected to diverge.

Related: pub type MovieOverrides = TitleOverrides keeps the old name alive for five in-tree call sites (policy.rs:58, policy.rs:106, search.rs:6, search.rs:296, search.rs:463). Nothing outside this workspace consumes it, so the alias can go with a rename in the same pass.

`MovieState` and `EpisodeState` are now the same two-line enum twice over, which is the duplication `TitleOverrides` just removed one instance of. Worth the same treatment, or a comment saying the two are expected to diverge. Related: `pub type MovieOverrides = TitleOverrides` keeps the old name alive for five in-tree call sites (`policy.rs:58`, `policy.rs:106`, `search.rs:6`, `search.rs:296`, `search.rs:463`). Nothing outside this workspace consumes it, so the alias can go with a rename in the same pass.

naps62-yolo marked this conversation as resolved

						
				@@ -0,0 +67,4 @@

				    wanted           INTEGER NOT NULL DEFAULT 0 CHECK (wanted IN (0, 1)),

				    -- Mirrors arr-core's canonical EpisodeState vocabulary.

				    state            TEXT NOT NULL DEFAULT 'missing'

				                     CHECK (state IN ('missing', 'downloading', 'available')),

naps62-yolo commented

Episodes now mirror EpisodeState, but movies.state in 0001_movies.sql is still CHECK (state IN ('missing', 'grabbed', 'imported')). One database, two vocabularies for the same concept, and 0001's comment — "arr-core (#7) owns the canonical enum this mirrors" — is now false for the table it sits on.

Either bring movies.state across in this migration (it is a rewrite of the CHECK plus an UPDATE mapping grabbed→downloading, imported→available), or open an issue and say here which table is the odd one out. Leaving it silent means the next reader has to diff two migrations to find out.

Episodes now mirror `EpisodeState`, but `movies.state` in `0001_movies.sql` is still `CHECK (state IN ('missing', 'grabbed', 'imported'))`. One database, two vocabularies for the same concept, and 0001's comment — "arr-core (#7) owns the canonical enum this mirrors" — is now false for the table it sits on. Either bring `movies.state` across in this migration (it is a rewrite of the CHECK plus an UPDATE mapping grabbed→downloading, imported→available), or open an issue and say here which table is the odd one out. Leaving it silent means the next reader has to diff two migrations to find out.

naps62-yolo marked this conversation as resolved

						
				@@ -0,0 +91,4 @@

				    json('{"dv_profile_allow":["8.1"],"dv_profile_reject":["5","7"],"allow_hdr10":true,"allow_hdr10plus":true,"allow_sdr":true}'),

				    json('{"2160p":{"floor_gib":8,"target_gib":22,"penalty_points_per_gib_over":60},"1080p":{"floor_gib":3,"target_gib":8,"penalty_points_per_gib_over":60}}'),

				    json('["2160p","1080p"]'),

				    json('{"Remux":4,"BluRay":3,"WEB-DL":2,"WEBRip":1,"HDTV":0}'),

naps62-yolo commented