From 269a5cbfda42273beb28b6b3b94fbc7012c1e663 Mon Sep 17 00:00:00 2001 From: twotalesanimation <80506065+twotalesanimation@users.noreply.github.com> Date: Tue, 1 Sep 2026 08:08:37 +0200 Subject: [PATCH] fix(delivery): separate packages by show, and number same-day deliveries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults in how a package's folder is named, both of which put unrelated files in one directory on the delivery volume. The default layout had no {showId}, so every show's episode 101 resolved to {deliveryRoot}/101/{packageName}. The token existed and was populated — it just never appeared in the path. With hardlinked packages that means two shows delivering the same episode number on the same day write into the same folder, and identically named shots overwrite each other. reviewDir had the same hole, and is worse there: reviewNameFor scopes its lookup by projectId, so a second show computes the bare base name and lands on the first show's folder instead of suffixing past it. createDeliveryPackage also never checked for an existing package of the same name. packageNameTemplate is date-only and packageNumber is a per-project counter that is not in the template, so a second delivery on the same day got an identical name and rootPath and built on top of the first. MOV reviews already did this correctly, so nextReviewName moves to deliveries.ts as nextPackageName and both paths share it; the review-side name is kept as an alias because it reads better at that call site. Putting {showId} ahead of {episode} exposes an empty segment on non-episodic shows, so paths go through tidyPath, which collapses it and leaves the UNC root alone. Historic packages are unaffected: buildDeliveryJobManifest reads rootPath and configSnapshot off the record, so only new packages use the new layout. A project with its own deliveryConfig.layout in the database still overrides these defaults. Co-Authored-By: Claude Opus 5 --- lib/render-pipeline/deliveries.test.ts | 65 ++++++++++++++++++++++++++ lib/render-pipeline/deliveries.ts | 49 +++++++++++++++++-- lib/render-pipeline/reviews.ts | 25 +++------- 3 files changed, 117 insertions(+), 22 deletions(-) create mode 100644 lib/render-pipeline/deliveries.test.ts diff --git a/lib/render-pipeline/deliveries.test.ts b/lib/render-pipeline/deliveries.test.ts new file mode 100644 index 0000000..7c81eb7 --- /dev/null +++ b/lib/render-pipeline/deliveries.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import { DELIVERY_CONFIG_DEFAULT, applyTokens, nextPackageName, tidyPath } from "./deliveries"; + +const BASE = "260901_Delivery"; + +describe("nextPackageName", () => { + it("uses the bare base for the first package of the day", () => { + expect(nextPackageName(BASE, [])).toBe(BASE); + }); + + it("suffixes later packages sharing the base", () => { + expect(nextPackageName(BASE, [BASE])).toBe(`${BASE}_02`); + expect(nextPackageName(BASE, [BASE, `${BASE}_02`])).toBe(`${BASE}_03`); + }); + + it("never reuses a number after a gap", () => { + expect(nextPackageName(BASE, [BASE, `${BASE}_02`, `${BASE}_04`])).toBe(`${BASE}_05`); + }); + + it("ignores names that only share a prefix", () => { + expect(nextPackageName(BASE, [`${BASE}_final`, "260902_Delivery"])).toBe(BASE); + }); +}); + +describe("delivery layout", () => { + const tokens = { + deliveryRoot: "//SAN/DELIVERY", + showId: "ACME", + episode: "101", + packageName: BASE, + }; + + it("separates shows so two shows' episode 101 cannot collide", () => { + const acme = applyTokens(DELIVERY_CONFIG_DEFAULT.layout.packageDir, tokens); + const other = applyTokens(DELIVERY_CONFIG_DEFAULT.layout.packageDir, { ...tokens, showId: "BRAVO" }); + expect(acme).toBe("//SAN/DELIVERY/ACME/101/260901_Delivery"); + expect(other).not.toBe(acme); + }); + + it("puts producer reviews under the show too", () => { + expect(applyTokens(DELIVERY_CONFIG_DEFAULT.layout.reviewDir, tokens)).toBe( + "//SAN/DELIVERY/ACME/PRODUCER_REVIEWS/260901_Delivery" + ); + }); +}); + +describe("tidyPath", () => { + it("drops the empty segment a non-episodic show leaves behind", () => { + const raw = applyTokens(DELIVERY_CONFIG_DEFAULT.layout.packageDir, { + deliveryRoot: "//SAN/DELIVERY", + showId: "ACME", + episode: "", + packageName: BASE, + }); + expect(tidyPath(raw)).toBe("//SAN/DELIVERY/ACME/260901_Delivery"); + }); + + it("keeps the UNC root intact", () => { + expect(tidyPath("//SAN/DELIVERY/ACME")).toBe("//SAN/DELIVERY/ACME"); + }); + + it("leaves a well-formed path alone", () => { + expect(tidyPath("V:/DELIVERY/ACME/101/x")).toBe("V:/DELIVERY/ACME/101/x"); + }); +}); diff --git a/lib/render-pipeline/deliveries.ts b/lib/render-pipeline/deliveries.ts index b19a70a..7d7cded 100644 --- a/lib/render-pipeline/deliveries.ts +++ b/lib/render-pipeline/deliveries.ts @@ -46,10 +46,12 @@ export const DELIVERY_CONFIG_DEFAULT: DeliveryConfig = { packageNameTemplate: "{yymmdd}_Delivery", reviewNameTemplate: "{yyyymmdd}_Producer_review", layout: { - packageDir: "{deliveryRoot}/{episode}/{packageName}", + // {showId} is not optional in practice: without it every show's episode 101 + // lands in one folder on the delivery volume. + packageDir: "{deliveryRoot}/{showId}/{episode}/{packageName}", shotDir: "{packageDir}/{shotCode}", reportFile: "{packageDir}/{packageName}_report.csv", - reviewDir: "{deliveryRoot}/PRODUCER_REVIEWS/{packageName}", + reviewDir: "{deliveryRoot}/{showId}/PRODUCER_REVIEWS/{packageName}", }, deliveryRoot: "//SAN/DELIVERY", includeMovs: true, @@ -108,6 +110,36 @@ export function applyTokens(template: string, tokens: Partial): str }); } +/** + * Collapse the empty segment an unset token leaves behind — a non-episodic + * show resolves {episode} to "" and would otherwise build ".../SHOW//NAME". + * The leading pair of a UNC root (//SAN/...) is preserved. + */ +export function tidyPath(path: string): string { + const uncPrefix = /^\/\//.test(path) ? "//" : ""; + return uncPrefix + path.slice(uncPrefix.length).replace(/\/{2,}/g, "/").replace(/\/+$/, ""); +} + +function escapeRegExp(value: string): string { + return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); +} + +/** + * Second and later packages sharing a base name get `_02`, `_03`, ... Numbering + * counts every package built under that base — cancelled and failed ones + * included — so a number is never reused for a different folder. + */ +export function nextPackageName(base: string, existingNames: string[]): string { + const pattern = new RegExp(`^${escapeRegExp(base)}(?:_(\\d+))?$`); + let highest = 0; + for (const name of existingNames) { + const match = pattern.exec(name); + if (!match) continue; + highest = Math.max(highest, match[1] ? Number.parseInt(match[1], 10) : 1); + } + return highest === 0 ? base : `${base}_${String(highest + 1).padStart(2, "0")}`; +} + export function dateTokens(date: Date) { const yyyy = String(date.getFullYear()); const mm = String(date.getMonth() + 1).padStart(2, "0"); @@ -280,8 +312,17 @@ export async function createDeliveryPackage(input: CreateDeliveryInput, userId: versionString: "", packageNumber: String(packageNumber), }; - tokens.packageName = input.options?.name ?? applyTokens(config.packageNameTemplate, tokens); - tokens.packageDir = applyTokens(config.layout.packageDir, tokens); + // A second delivery on the same day is normal — a retake batch, or a client + // asking for the rest of a sequence — and the date-only template gives it a + // name identical to the morning's. Suffix it the way MOV reviews already + // are, so it gets its own folder instead of merging into the first one. + const baseName = applyTokens(config.packageNameTemplate, tokens); + const sameBase = await tx.deliveryPackage.findMany({ + where: { projectId: project.id, kind: "EXR_DELIVERY", name: { startsWith: baseName } }, + select: { name: true }, + }); + tokens.packageName = input.options?.name ?? nextPackageName(baseName, sameBase.map((p) => p.name)); + tokens.packageDir = tidyPath(applyTokens(config.layout.packageDir, tokens)); const pkg = await tx.deliveryPackage.create({ data: { diff --git a/lib/render-pipeline/reviews.ts b/lib/render-pipeline/reviews.ts index d3dd5aa..a96ce3b 100644 --- a/lib/render-pipeline/reviews.ts +++ b/lib/render-pipeline/reviews.ts @@ -6,7 +6,9 @@ import { getConfigNumber } from "./config"; import { applyTokens, dateTokens, + nextPackageName, resolveDeliveryConfig, + tidyPath, type DeliveryConfig, type DeliveryManifest, type TokenValues, @@ -165,25 +167,12 @@ export async function resolveReviewCandidates(shotIds: string[]) { // ── Folder naming ──────────────────────────────────────────────────────────── -function escapeRegExp(value: string): string { - return value.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); -} - /** * `20260828_Producer_review`, then `_02`, `_03` for further reviews the same - * day. Numbering counts every package built under that base — cancelled and - * failed ones included — so a number is never reused for a different folder. + * day. EXR deliveries now number the same way, so the rule lives in + * deliveries.ts; this name is kept because it reads better at the call site. */ -export function nextReviewName(base: string, existingNames: string[]): string { - const pattern = new RegExp(`^${escapeRegExp(base)}(?:_(\\d+))?$`); - let highest = 0; - for (const name of existingNames) { - const match = pattern.exec(name); - if (!match) continue; - highest = Math.max(highest, match[1] ? Number.parseInt(match[1], 10) : 1); - } - return highest === 0 ? base : `${base}_${String(highest + 1).padStart(2, "0")}`; -} +export const nextReviewName = nextPackageName; async function reviewNameFor( tx: Prisma.TransactionClient | typeof db, @@ -243,7 +232,7 @@ export async function previewMovReviewPackage(shotIds: string[]) { const tokens = reviewTokens(config, project, commonEpisode(candidates), buildDate, 0); const base = applyTokens(config.reviewNameTemplate, tokens); tokens.packageName = await reviewNameFor(db, project.id, base); - tokens.packageDir = applyTokens(config.layout.reviewDir, tokens); + tokens.packageDir = tidyPath(applyTokens(config.layout.reviewDir, tokens)); return { projectId: project.id, @@ -299,7 +288,7 @@ export async function createMovReviewPackage(input: CreateMovReviewInput, userId const tokens = reviewTokens(config, project, episode, buildDate, packageNumber); const base = applyTokens(config.reviewNameTemplate, tokens); tokens.packageName = input.options?.name ?? (await reviewNameFor(tx, project.id, base)); - tokens.packageDir = applyTokens(config.layout.reviewDir, tokens); + tokens.packageDir = tidyPath(applyTokens(config.layout.reviewDir, tokens)); const pkg = await tx.deliveryPackage.create({ data: {