fix(playlists): "+N without length" counts every video the sum left out
The label said "without length" but the count held only live videos, so a video not fetched yet, or gone before it was, was left out of the sum and named nowhere. The server's `Video.without_length` and the client's `isWithoutLength` are now one rule (no length, whatever the live status), and the field, the key and the helpers are named for what they count. The comments that still quoted "+N live" say the current copy.
This commit is contained in:
@@ -17,7 +17,6 @@ from sqlalchemy import (
|
||||
String,
|
||||
Text,
|
||||
UniqueConstraint,
|
||||
and_,
|
||||
case,
|
||||
func,
|
||||
literal_column,
|
||||
@@ -125,7 +124,7 @@ LIVE_OR_UPCOMING = ("live", "upcoming")
|
||||
# the stream has run (measured on prod, 2026-11-06: 122,962,914 s), and one such video made a Watch
|
||||
# later total read 44,073 hours. YouTube caps an upload at 12 hours, so nothing real is lost. Such a
|
||||
# length is unknown wherever a length is used: the API sends none (`Video.length_seconds`), sums leave
|
||||
# it out (a playlist's counts it as `live_without_length`), and the duration filter, sort and Home budget never
|
||||
# it out (a playlist's counts it as `without_length`), and the duration filter, sort and Home budget never
|
||||
# match it. The raw `duration_seconds` stays for what it does prove — that the video is not a Short —
|
||||
# and for the jobs that ask whether YouTube has answered yet (test_video_length holds who reads it).
|
||||
LENGTH_MAX_S = 24 * 3600
|
||||
@@ -239,15 +238,16 @@ class Video(Base, TimestampMixin):
|
||||
)
|
||||
|
||||
@hybrid_property
|
||||
def live_without_length(self) -> bool:
|
||||
"""A live video (on air, upcoming, a premiere or an archive) whose length is unknown: what a
|
||||
total leaves out and names as "+N live"."""
|
||||
return self.live_status != "none" and self.length_seconds is None
|
||||
def without_length(self) -> bool:
|
||||
"""A video whose length is unknown — live, an archive past a day, or not fetched yet: what a
|
||||
sum of lengths leaves out and names as "+N without length". Every such video, so the count
|
||||
and the sum never disagree about what was left out."""
|
||||
return self.length_seconds is None
|
||||
|
||||
@live_without_length.inplace.expression
|
||||
@without_length.inplace.expression
|
||||
@classmethod
|
||||
def _live_without_length_expression(cls) -> ColumnElement[bool]:
|
||||
return and_(cls.live_status != "none", cls.length_seconds.is_(None))
|
||||
def _without_length_expression(cls) -> ColumnElement[bool]:
|
||||
return cls.length_seconds.is_(None)
|
||||
|
||||
# Weighted full-text search document (DB-generated, never written by the app): title (A) >
|
||||
# creator keywords + search queries (B) > truncated description (C). ts_rank honours the
|
||||
|
||||
@@ -147,15 +147,15 @@ def item_counts(db: Session, ids: list[int]) -> dict[int, int]:
|
||||
|
||||
|
||||
def durations(db: Session, ids: list[int]) -> dict[int, tuple[int, int]]:
|
||||
"""Each playlist's running time — the sum of its videos' known lengths — and how many live videos
|
||||
it cannot hold (`Video.live_without_length`)."""
|
||||
"""Each playlist's running time — the sum of its videos' known lengths — and how many videos
|
||||
it left out for having none (`Video.without_length`)."""
|
||||
return {
|
||||
pid: (int(total or 0), int(live or 0))
|
||||
for pid, total, live in db.execute(
|
||||
pid: (int(total or 0), int(left_out or 0))
|
||||
for pid, total, left_out in db.execute(
|
||||
select(
|
||||
PlaylistItem.playlist_id,
|
||||
func.coalesce(func.sum(Video.length_seconds), 0),
|
||||
func.count().filter(Video.live_without_length),
|
||||
func.count().filter(Video.without_length),
|
||||
)
|
||||
.join(Video, Video.id == PlaylistItem.video_id)
|
||||
.where(PlaylistItem.playlist_id.in_(ids))
|
||||
@@ -197,14 +197,14 @@ def covers(db: Session, ids: list[int]) -> dict[int, str | None]:
|
||||
|
||||
|
||||
def playlist_totals(db: Session, playlist_id: int) -> tuple[int, int, int, str | None]:
|
||||
"""One playlist's item count, running time, live videos the running time cannot hold
|
||||
(`Video.live_without_length`) and cover. An outer join, so an empty list still answers
|
||||
"""One playlist's item count, running time, the videos the running time left out for having no
|
||||
length (`Video.without_length`) and cover. An outer join, so an empty list still answers
|
||||
(0, 0, 0, None)."""
|
||||
count, total, live = db.execute(
|
||||
count, total, left_out = db.execute(
|
||||
select(
|
||||
func.count(PlaylistItem.id),
|
||||
func.coalesce(func.sum(Video.length_seconds), 0),
|
||||
func.count().filter(Video.live_without_length),
|
||||
func.count().filter(Video.without_length),
|
||||
)
|
||||
.select_from(PlaylistItem)
|
||||
.outerjoin(Video, Video.id == PlaylistItem.video_id)
|
||||
@@ -217,7 +217,7 @@ def playlist_totals(db: Session, playlist_id: int) -> tuple[int, int, int, str |
|
||||
.order_by(PlaylistItem.position)
|
||||
.limit(1)
|
||||
)
|
||||
return count or 0, total or 0, live or 0, cover
|
||||
return count or 0, total or 0, left_out or 0, cover
|
||||
|
||||
|
||||
def playlist_rows(db: Session, user: User, playlist_id: int) -> list:
|
||||
|
||||
@@ -99,9 +99,9 @@ def _summary(
|
||||
has_video: bool | None = None,
|
||||
cover: str | None = None,
|
||||
) -> dict:
|
||||
"""`duration` is `queries.durations`'s pair: the known running time, and the live videos it
|
||||
cannot hold — the "+N live" beside it."""
|
||||
total_duration, live = duration
|
||||
"""`duration` is `queries.durations`'s pair: the known running time, and the videos it left out
|
||||
for having no length — the "+N without length" beside it."""
|
||||
total_duration, left_out = duration
|
||||
out = {
|
||||
"id": pl.id,
|
||||
"name": pl.name,
|
||||
@@ -111,7 +111,7 @@ def _summary(
|
||||
"dirty": pl.dirty,
|
||||
"item_count": count,
|
||||
"total_duration_seconds": total_duration,
|
||||
"live_without_length": live,
|
||||
"without_length": left_out,
|
||||
"cover_thumbnail": cover,
|
||||
# When this list last CHANGED (rename, add/remove/move/reorder) — the rail's
|
||||
# "recently changed first" sort reads it. See `service.touch` for why it isn't automatic.
|
||||
@@ -369,8 +369,8 @@ def rename_playlist(
|
||||
raise _refused(400, "name_required")
|
||||
service.rename_playlist(db, pl, name)
|
||||
db.commit()
|
||||
count, total, live, cover = queries.playlist_totals(db, pl.id)
|
||||
return _summary(pl, count, (total, live), cover=cover)
|
||||
count, total, left_out, cover = queries.playlist_totals(db, pl.id)
|
||||
return _summary(pl, count, (total, left_out), cover=cover)
|
||||
|
||||
|
||||
@router.delete("/{playlist_id}")
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
Measured on prod (2026-11-06): two 24/7 streams' archives report 122,962,914 s and 34,762,459 s, and the
|
||||
owner's Watch later total read "44,073 hr 55 min". Such a video now has no length on any API answer, the
|
||||
totals leave it out and count it as `live_without_length`, and the duration filter and sort pass it over
|
||||
totals leave it out (a playlist's counts it as `without_length`), and the duration filter and sort pass it over
|
||||
like a video whose length is not fetched yet. An AST check holds who may still read the raw column.
|
||||
DB lane (the AST check needs none).
|
||||
"""
|
||||
@@ -30,7 +30,7 @@ def _seed(db, user):
|
||||
("day", LENGTH_MAX_S, "was_live"), # exactly a day still is a length
|
||||
("archive", YEARS, "was_live"),
|
||||
("onair", None, "live"),
|
||||
("unfetched", None, "none"), # not live: not counted as "+N live"
|
||||
("unfetched", None, "none"), # not live, but left out of the sum: counted all the same
|
||||
]
|
||||
for i, (vid, seconds, live) in enumerate(rows):
|
||||
db.add(
|
||||
@@ -121,7 +121,7 @@ def test_an_off_catalog_archive_has_no_length_either(db, make_user, as_user, mon
|
||||
assert client.get("/api/videos/offshort").json()["duration_seconds"] == HOUR
|
||||
|
||||
|
||||
def test_the_totals_leave_it_out_and_count_the_live_ones(db, make_user, as_user):
|
||||
def test_the_totals_leave_it_out_and_count_what_they_left_out(db, make_user, as_user):
|
||||
user = make_user()
|
||||
pl = _seed(db, user)
|
||||
client = as_user(user)
|
||||
@@ -130,19 +130,19 @@ def test_the_totals_leave_it_out_and_count_the_live_ones(db, make_user, as_user)
|
||||
assert listed.status_code == 200
|
||||
(summary,) = [p for p in listed.json() if p["id"] == pl.id]
|
||||
assert summary["total_duration_seconds"] == KNOWN_SUM
|
||||
assert summary["live_without_length"] == 2 # the archive and the one on air
|
||||
assert summary["without_length"] == 3 # the archive, the one on air, the unfetched one
|
||||
|
||||
renamed = client.patch(f"/api/playlists/{pl.id}", json={"name": "Later 2"})
|
||||
assert renamed.status_code == 200
|
||||
assert renamed.json()["total_duration_seconds"] == KNOWN_SUM
|
||||
assert renamed.json()["live_without_length"] == 2
|
||||
assert renamed.json()["without_length"] == 3
|
||||
|
||||
channels = client.get("/api/channels")
|
||||
assert channels.status_code == 200
|
||||
(ch,) = channels.json()
|
||||
assert ch["total_duration_seconds"] == KNOWN_SUM
|
||||
# The channel table shows the sum alone (the count did not fit its column), so it is not sent.
|
||||
assert "live_without_length" not in ch
|
||||
assert "without_length" not in ch and "live_without_length" not in ch
|
||||
|
||||
|
||||
def test_the_duration_filter_and_sort_pass_it_over(db, make_user, as_user):
|
||||
|
||||
@@ -1,21 +0,0 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { TFunction } from "i18next";
|
||||
import { isLiveWithoutLength, withLiveCount } from "./liveWithoutLength";
|
||||
|
||||
const t = ((key: string, o: { count: number }) => `${key}:${o.count}`) as unknown as TFunction;
|
||||
|
||||
describe("a total and the live videos it cannot hold", () => {
|
||||
it("counts a live video only when the server sent no length", () => {
|
||||
expect(isLiveWithoutLength({ duration_seconds: null, live_status: "was_live" })).toBe(true);
|
||||
expect(isLiveWithoutLength({ duration_seconds: null, live_status: "live" })).toBe(true);
|
||||
expect(isLiveWithoutLength({ duration_seconds: 5400, live_status: "was_live" })).toBe(false);
|
||||
// Not fetched yet, and not live: left out of the sum like before, not named as live.
|
||||
expect(isLiveWithoutLength({ duration_seconds: null, live_status: "none" })).toBe(false);
|
||||
});
|
||||
|
||||
it("names them beside the sum, or alone when nothing summed", () => {
|
||||
expect(withLiveCount("12 hr 5 min", 2, t)).toBe("12 hr 5 min card.liveWithoutLength:2");
|
||||
expect(withLiveCount("", 1, t)).toBe("card.liveWithoutLength:1");
|
||||
expect(withLiveCount("12 hr 5 min", 0, t)).toBe("12 hr 5 min");
|
||||
});
|
||||
});
|
||||
@@ -1,18 +0,0 @@
|
||||
// A total of video lengths and the live videos it cannot hold. The server decides what a length is
|
||||
// (a YouTube "length" past a day is a 24/7 stream's archive reporting the years it has run, and the
|
||||
// API sends none for it: `catalog.models.LENGTH_MAX_S`), so this side keeps no copy of that limit. It
|
||||
// only names what a sum left out: a live video — on air, upcoming, a premiere or an archive — with no
|
||||
// length, as the server's `Video.live_without_length` counts it for the totals it sums itself.
|
||||
import type { TFunction } from "i18next";
|
||||
import type { Video } from "../../lib/api/feed";
|
||||
|
||||
export function isLiveWithoutLength(v: Pick<Video, "duration_seconds" | "live_status">): boolean {
|
||||
return v.duration_seconds == null && v.live_status !== "none";
|
||||
}
|
||||
|
||||
/** "12 hr 5 min +2 without length": `total` is the formatted sum ("" when nothing summed), `live` the count
|
||||
* it left out. Empty when both are. */
|
||||
export function withLiveCount(total: string, live: number, t: TFunction): string {
|
||||
const suffix = live > 0 ? t("card.liveWithoutLength", { count: live }) : "";
|
||||
return [total, suffix].filter(Boolean).join(" ");
|
||||
}
|
||||
@@ -0,0 +1,19 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { TFunction } from "i18next";
|
||||
import { isWithoutLength, withLeftOutCount } from "./withoutLength";
|
||||
|
||||
const t = ((key: string, o: { count: number }) => `${key}:${o.count}`) as unknown as TFunction;
|
||||
|
||||
describe("a total and the videos it cannot hold", () => {
|
||||
it("counts every video the server sent no length for", () => {
|
||||
expect(isWithoutLength({ duration_seconds: null })).toBe(true);
|
||||
expect(isWithoutLength({ duration_seconds: 5400 })).toBe(false);
|
||||
expect(isWithoutLength({ duration_seconds: 0 })).toBe(false);
|
||||
});
|
||||
|
||||
it("names them beside the sum, or alone when nothing summed", () => {
|
||||
expect(withLeftOutCount("12 hr 5 min", 2, t)).toBe("12 hr 5 min card.withoutLength:2");
|
||||
expect(withLeftOutCount("", 1, t)).toBe("card.withoutLength:1");
|
||||
expect(withLeftOutCount("12 hr 5 min", 0, t)).toBe("12 hr 5 min");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,19 @@
|
||||
// A total of video lengths and the videos it cannot hold. The server decides what a length is (a
|
||||
// YouTube "length" past a day is a 24/7 stream's archive reporting the years it has run, and the API
|
||||
// sends none for it: `catalog.models.LENGTH_MAX_S`), so this side keeps no copy of that limit. It only
|
||||
// names what a sum left out: every video with no length — live, an archive, or not fetched yet — as the
|
||||
// server's `Video.without_length` counts it for the totals it sums itself. The two rules are the same,
|
||||
// so the count and the sum never disagree about what was left out.
|
||||
import type { TFunction } from "i18next";
|
||||
import type { Video } from "../../lib/api/feed";
|
||||
|
||||
export function isWithoutLength(v: Pick<Video, "duration_seconds">): boolean {
|
||||
return v.duration_seconds == null;
|
||||
}
|
||||
|
||||
/** "12 hr 5 min +2 without length": `total` is the formatted sum ("" when nothing summed), `leftOut`
|
||||
* the count it left out. Empty when both are. */
|
||||
export function withLeftOutCount(total: string, leftOut: number, t: TFunction): string {
|
||||
const suffix = leftOut > 0 ? t("card.withoutLength", { count: leftOut }) : "";
|
||||
return [total, suffix].filter(Boolean).join(" ");
|
||||
}
|
||||
@@ -16,8 +16,8 @@
|
||||
"restart": "Restart",
|
||||
"play": "Play",
|
||||
"stream": "stream",
|
||||
"liveWithoutLength_one": "+{{count}} without length",
|
||||
"liveWithoutLength_other": "+{{count}} without length",
|
||||
"withoutLength_one": "+{{count}} without length",
|
||||
"withoutLength_other": "+{{count}} without length",
|
||||
"views": "views",
|
||||
"info": "Details",
|
||||
"noDescription": "No description",
|
||||
|
||||
@@ -16,8 +16,8 @@
|
||||
"restart": "Elölről",
|
||||
"play": "Lejátszás",
|
||||
"stream": "közvetítés",
|
||||
"liveWithoutLength_one": "+{{count}} hossz nélkül",
|
||||
"liveWithoutLength_other": "+{{count}} hossz nélkül",
|
||||
"withoutLength_one": "+{{count}} hossz nélkül",
|
||||
"withoutLength_other": "+{{count}} hossz nélkül",
|
||||
"views": "megtekintés",
|
||||
"info": "Részletek",
|
||||
"noDescription": "Nincs leírás",
|
||||
|
||||
@@ -17,8 +17,8 @@ export interface Playlist {
|
||||
dirty: boolean; // linked local playlist has edits not yet pushed to YouTube
|
||||
item_count: number;
|
||||
total_duration_seconds: number;
|
||||
/** Live videos the total cannot hold (no length): the "+N without length" beside it. */
|
||||
live_without_length: number;
|
||||
/** The videos the total left out for having no length: the "+N without length" beside it. */
|
||||
without_length: number;
|
||||
cover_thumbnail: string | null;
|
||||
updated_at: string; // ISO; when the list last CHANGED (rename, add/remove/move/reorder)
|
||||
has_video?: boolean; // only set when listed with ?contains=<video_id>
|
||||
|
||||
@@ -192,10 +192,10 @@ describe("buildChannelColumns — Added", () => {
|
||||
const length = cols.find((c) => c.key === "length");
|
||||
const row = {
|
||||
...ch({ total_duration_seconds: 7200 }),
|
||||
live_without_length: 2,
|
||||
without_length: 2,
|
||||
} as ManagedChannel;
|
||||
const { container } = render(<>{length!.render(row)}</>);
|
||||
expect(container.textContent).not.toContain("card.liveWithoutLength");
|
||||
expect(container.textContent).not.toContain("card.withoutLength");
|
||||
expect(container.querySelectorAll("span")).toHaveLength(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -990,24 +990,25 @@ describe("Playlists · the header", () => {
|
||||
)
|
||||
).textContent;
|
||||
|
||||
it("sums what is known, and says nothing about a runtime nothing sums to", async () => {
|
||||
it("sums what is known, and names what it left out beside it", async () => {
|
||||
// Not fetched yet, and not live: left out of the sum all the same, so it is named.
|
||||
mount({ 1: [vid("A", 0), untimed("B")] });
|
||||
expect(await meta()).toBe("playlists.itemCount · 50 min");
|
||||
expect(await meta()).toBe("playlists.itemCount · 50 min card.withoutLength");
|
||||
cleanup();
|
||||
|
||||
mount({ 1: [untimed("C")] });
|
||||
expect(await meta()).toBe("playlists.itemCount");
|
||||
mount({ 1: [vid("A", 0)] });
|
||||
expect(await meta()).toBe("playlists.itemCount · 50 min");
|
||||
});
|
||||
|
||||
it("names the live videos the sum cannot hold beside it", async () => {
|
||||
it("names a live video the sum cannot hold, alone when nothing summed", async () => {
|
||||
// A 24/7 stream's archive comes with no length (the server's rule): it is named, not summed.
|
||||
const archive = (id: string) => ({ ...untimed(id), live_status: "was_live" }) as Video;
|
||||
mount({ 1: [vid("A", 0), archive("B")] });
|
||||
expect(await meta()).toBe("playlists.itemCount · 50 min card.liveWithoutLength");
|
||||
expect(await meta()).toBe("playlists.itemCount · 50 min card.withoutLength");
|
||||
cleanup();
|
||||
|
||||
mount({ 1: [archive("C")] });
|
||||
expect(await meta()).toBe("playlists.itemCount · card.liveWithoutLength");
|
||||
expect(await meta()).toBe("playlists.itemCount · card.withoutLength");
|
||||
});
|
||||
|
||||
it("offers the view controls only once there is more than one row to arrange", async () => {
|
||||
|
||||
@@ -68,7 +68,7 @@ import {
|
||||
playlistViewParamUpdates,
|
||||
} from "../../lib/playlistsUrlState";
|
||||
import { formatRuntime } from "../../lib/format";
|
||||
import { isLiveWithoutLength, withLiveCount } from "../../features/video/liveWithoutLength";
|
||||
import { isWithoutLength, withLeftOutCount } from "../../features/video/withoutLength";
|
||||
import { applyView, sameOrder } from "../../lib/playlistView";
|
||||
import { useOpenLinkedPlayer } from "../../features/player/useOpenLinkedPlayer";
|
||||
const PlayerModal = lazy(() => import("../../features/player/PlayerModal"));
|
||||
@@ -566,13 +566,13 @@ function PlaylistDetail() {
|
||||
}, [items]);
|
||||
|
||||
// Total playtime of the whole playlist (sums every item, matching the itemCount beside it — not
|
||||
// the filtered `shown` set). Items with an unknown duration count as 0, and a live one is named
|
||||
// beside the sum instead ("+2 without length"); hidden when there is neither.
|
||||
// the filtered `shown` set). An item with an unknown duration is left out of the sum and named
|
||||
// beside it instead ("+2 without length"); hidden when there is neither.
|
||||
const totalSeconds = useMemo(
|
||||
() => items.reduce((acc, v) => acc + (v.duration_seconds ?? 0), 0),
|
||||
[items],
|
||||
);
|
||||
const liveCount = useMemo(() => items.filter(isLiveWithoutLength).length, [items]);
|
||||
const leftOutCount = useMemo(() => items.filter(isWithoutLength).length, [items]);
|
||||
|
||||
// The row's "…" opens the row menu under itself. Which row's menu is open is part of the
|
||||
// cell (aria-expanded), so the columns rebuild when it changes — a cheap array of closures.
|
||||
@@ -1622,8 +1622,8 @@ function PlaylistDetail() {
|
||||
>
|
||||
<span className="text-xs text-muted">
|
||||
{t("playlists.itemCount", { count: items.length })}
|
||||
{(totalSeconds > 0 || liveCount > 0) &&
|
||||
` · ${withLiveCount(formatRuntime(totalSeconds, lang), liveCount, t)}`}
|
||||
{(totalSeconds > 0 || leftOutCount > 0) &&
|
||||
` · ${withLeftOutCount(formatRuntime(totalSeconds, lang), leftOutCount, t)}`}
|
||||
{filterActive &&
|
||||
` · ${t("playlists.filteredCount", { count: shown.length })}`}
|
||||
</span>
|
||||
|
||||
@@ -75,7 +75,7 @@ const pl = (id: number, name: string) =>
|
||||
yt_playlist_id: null,
|
||||
item_count: 1,
|
||||
total_duration_seconds: 0,
|
||||
live_without_length: 0,
|
||||
without_length: 0,
|
||||
cover_thumbnail: null,
|
||||
dirty: false,
|
||||
}) as unknown as Playlist;
|
||||
|
||||
@@ -80,7 +80,7 @@ const alpha = {
|
||||
source: "local",
|
||||
item_count: 3,
|
||||
total_duration_seconds: 0,
|
||||
live_without_length: 0,
|
||||
without_length: 0,
|
||||
cover_thumbnail: null,
|
||||
dirty: false,
|
||||
} as unknown as Playlist;
|
||||
|
||||
@@ -8,7 +8,7 @@ import { YouTubeIcon } from "../../lib/brandIcons";
|
||||
import { api, type Playlist } from "../../lib/api";
|
||||
import { compareText } from "../../lib/collate";
|
||||
import { formatRuntime } from "../../lib/format";
|
||||
import { withLiveCount } from "../../features/video/liveWithoutLength";
|
||||
import { withLeftOutCount } from "../../features/video/withoutLength";
|
||||
import { msg, notify } from "../../lib/notifications";
|
||||
import { notifyYouTubeActionError } from "../../lib/youtubeErrors";
|
||||
import { LS } from "../../lib/storage";
|
||||
@@ -379,10 +379,10 @@ export default function PlaylistsRail() {
|
||||
</div>
|
||||
<div className="text-2xs text-muted">
|
||||
{t("playlists.itemCount", { count: pl.item_count })}
|
||||
{(pl.total_duration_seconds > 0 || pl.live_without_length > 0) &&
|
||||
` · ${withLiveCount(
|
||||
{(pl.total_duration_seconds > 0 || pl.without_length > 0) &&
|
||||
` · ${withLeftOutCount(
|
||||
formatRuntime(pl.total_duration_seconds, lang),
|
||||
pl.live_without_length,
|
||||
pl.without_length,
|
||||
t,
|
||||
)}`}
|
||||
</div>
|
||||
|
||||
Reference in New Issue
Block a user