diff --git a/.changelog/next/fixed-issue-4149.md b/.changelog/next/fixed-issue-4149.md new file mode 100644 index 0000000000..f7f981f126 --- /dev/null +++ b/.changelog/next/fixed-issue-4149.md @@ -0,0 +1 @@ +- Creative Commission detail page now live-updates a freshly-run render instead of asking you to reload — it polls the referenced Creative Director projects while any run is still generating, and stops once they settle diff --git a/client/src/pages/CreativeCommissionDetail.jsx b/client/src/pages/CreativeCommissionDetail.jsx index bb19de48e1..1c018d8642 100644 --- a/client/src/pages/CreativeCommissionDetail.jsx +++ b/client/src/pages/CreativeCommissionDetail.jsx @@ -10,6 +10,12 @@ * generation config. The URL is the source of truth for what's open (the * ID-based deep-linking rule), so a render or its detail page is directly * shareable, bookmarkable, and reachable from ⌘K / voice / notification links. + * + * A run's render materializes ASYNCHRONOUSLY — the fire creates the Creative + * Director project and returns, then the planner/render loop fills it in over + * the following minutes. The page therefore polls the referenced projects while + * any of them is still generating (#4149) so a freshly-fired render appears in + * place; there is no server-side completion event to subscribe to today. */ import { useEffect, useMemo, useRef, useState, useCallback } from 'react'; @@ -19,6 +25,7 @@ import PageSkeleton from '../components/ui/PageSkeleton'; import toast from '../components/ui/Toast'; import ConfirmButtonPair from '../components/ui/ConfirmButtonPair'; import { useConfirmDelete } from '../hooks/useConfirmDelete'; +import { useAutoRefetch } from '../hooks/useAutoRefetch'; import { timeAgo } from '../utils/formatters'; import CommissionConfigForm from '../components/creative-commission/CommissionConfigForm.jsx'; import RenderHistory from '../components/creative-commission/RenderHistory.jsx'; @@ -30,6 +37,24 @@ import { submitCommissionFeedback, runCommissionNow, getCreativeDirectorProjectsByIds, } from '../services/api'; +// A CD project's lifecycle status is the only completion signal this page can +// read (no socket channel, and a commission run row is written once and never +// updated). These are the statuses where more output can still show up. +// +// NOTE the difference from `CreativeDirectorDetail`'s terminal set, which counts +// 'draft' as settled: there, a draft is a project the user hasn't started. Here, +// a commission fire creates the project and advances it in the same breath, so a +// draft we observe is just the sliver before the planner's first status write. +const GENERATING_PROJECT_STATUSES = new Set(['draft', 'planning', 'rendering', 'stitching']); + +// Hard ceiling on how long a `started` run is treated as still in flight. A run +// whose project stalled (crashed mid-plan) or was pruned would otherwise poll +// forever on a tab left open — bounding by run age stops that without needing a +// timer, and generously outlasts any real generation. +const IN_FLIGHT_RUN_MAX_AGE_MS = 6 * 60 * 60 * 1000; + +const PROJECT_POLL_MS = 5000; + export default function CreativeCommissionDetail() { const navigate = useNavigate(); const location = useLocation(); @@ -49,6 +74,16 @@ export default function CreativeCommissionDetail() { const [running, setRunning] = useState(false); const [projectsById, setProjectsById] = useState(() => new Map()); const [projectsLoading, setProjectsLoading] = useState(false); + // When the project batch was last ATTEMPTED — the clock the in-flight run + // check reads, so it re-evaluates on every poll tick rather than freezing at + // whatever `Date.now()` was during the last render. + const [projectsFetchedAt, setProjectsFetchedAt] = useState(0); + // The id set the batch last resolved SUCCESSFULLY. Distinct from the + // attempted-key ref below: a failed fetch is an attempt but not a load, and + // the in-flight check has to keep the two apart — an id missing from a + // successful batch is a pruned project (settled), while the same id missing + // because the request failed is simply not known yet (retry). + const [projectsLoadedKey, setProjectsLoadedKey] = useState(null); const { isConfirming, requestDelete, cancelDelete, confirmDelete } = useConfirmDelete(); // Load (and refresh) the deep-linked commission. `location.key` is a dep so a @@ -99,23 +134,82 @@ export default function CreativeCommissionDetail() { return [...new Set(ids)].sort().join(','); }, [commission]); - useEffect(() => { - if (!projectIdsKey) { setProjectsById(new Map()); setProjectsLoading(false); return; } - let cancelled = false; - setProjectsLoading(true); - getCreativeDirectorProjectsByIds(projectIdsKey.split(','), { silent: true }) - .then((projects) => { - if (cancelled) return; - // Index by id, not position: an id that no longer resolves (pruned - // project) is absent from the response, and its card degrades to the - // status-only placeholder. - setProjectsById(new Map((Array.isArray(projects) ? projects : []).map((p) => [p.id, p]))); - }) - .catch(() => { /* status-only cards degrade gracefully */ }) - .finally(() => { if (!cancelled) setProjectsLoading(false); }); - return () => { cancelled = true; }; + // ONE fetch path, shared by the id-set change and the generation poll below + // (#4149). `seq` supersedes an in-flight response another call has already + // replaced — a poll tick that resolves after the id set changed must not + // reinstate the previous set's projects. + const projectsFetchSeqRef = useRef(0); + const projectsAttemptedKeyRef = useRef(null); + const fetchProjects = useCallback(async () => { + const seq = (projectsFetchSeqRef.current += 1); + if (!projectIdsKey) { + setProjectsById(new Map()); + setProjectsLoading(false); + projectsAttemptedKeyRef.current = ''; + setProjectsLoadedKey(''); + return null; + } + // Only the FIRST attempt at a given id set shows "loading…" — a poll tick + // must not flash an already-resolved (or known-pruned) card back to the + // loading placeholder every few seconds. + if (projectsAttemptedKeyRef.current !== projectIdsKey) setProjectsLoading(true); + const projects = await getCreativeDirectorProjectsByIds(projectIdsKey.split(','), { silent: true }) + .catch(() => null); // null = fetch failed; [] = resolved-but-empty (keep the two apart) + if (seq !== projectsFetchSeqRef.current) return null; + // Index by id, not position: an id that no longer resolves (pruned project) + // is absent from the response, and its card degrades to the status-only + // placeholder. A FAILED fetch keeps the last good map instead of blanking it. + if (Array.isArray(projects)) { + setProjectsById(new Map(projects.map((p) => [p.id, p]))); + setProjectsLoadedKey(projectIdsKey); + } + projectsAttemptedKeyRef.current = projectIdsKey; + setProjectsLoading(false); + // Stamped on every ATTEMPT, not just a successful one, so the in-flight age + // bound below keeps re-evaluating even while the endpoint is failing. + setProjectsFetchedAt(Date.now()); + return null; }, [projectIdsKey]); + // Is any run still producing? A run row is written once with status 'started' + // and never revisited, so "still generating" has to come from the project it + // points at, in a non-terminal CD status. + const hasGeneratingRun = useMemo(() => { + // The freshness of the data we're judging, not wall-clock render time. + const now = projectsFetchedAt || Date.now(); + // Has the CURRENT id set come back from a successful batch? Until it has, an + // unresolved id means "not known yet" (initial load, or a failed attempt + // worth retrying) — after it has, the same id means the project was pruned, + // and no amount of polling brings it back. + const batchIsAuthoritative = projectsLoadedKey === projectIdsKey; + return (commission?.runs || []).some((r) => { + if (r?.status !== 'started' || !r.projectId) return false; + const ranAt = Date.parse(r.ranAt); + if (!Number.isFinite(ranAt) || now - ranAt > IN_FLIGHT_RUN_MAX_AGE_MS) return false; + const project = projectsById.get(r.projectId); + if (!project) return !batchIsAuthoritative; + return GENERATING_PROJECT_STATUSES.has(project.status); + }); + }, [commission, projectsById, projectsFetchedAt, projectsLoadedKey, projectIdsKey]); + + // Poll only while something is actually generating; the hook also pauses while + // the tab is hidden and re-fires on return. `immediate: false` because the + // id-set effect below already owns the first fetch. + const { refetch: refetchProjects } = useAutoRefetch(fetchProjects, PROJECT_POLL_MS, { + enabled: hasGeneratingRun, + immediate: false, + pollOnly: true, + }); + + // Fetch whenever the referenced id set changes (including the initial load and + // a Run Now appending a render) — the poll is gated on in-flight work, so it + // can't be responsible for the first read. The newest run id is a dep too: a + // fire always mints a NEW project today, so the id set moves on its own, but a + // run that ever REUSED a project id would otherwise be judged against the + // cached 'complete' snapshot and never start polling. + const latestRunId = commission?.runs?.length ? commission.runs[commission.runs.length - 1].id : null; + useEffect(() => { refetchProjects(); }, [projectIdsKey, latestRunId, refetchProjects]); + const patchForm = useCallback((path, value) => setForm((prev) => patchFormState(prev, path, value)), []); const handleSave = async () => { @@ -167,7 +261,7 @@ export default function CreativeCommissionDetail() { const fresh = result.commission; setCommission((prev) => (prev ? { ...prev, runs: fresh.runs, feedback: fresh.feedback } : fresh)); } - if (result?.status === 'started') toast.success('Run started — its render appears below once generation finishes (reload to refresh)'); + if (result?.status === 'started') toast.success('Run started — its render appears below once generation finishes'); else if (result?.status === 'skipped') toast.error(`Run skipped: ${result.reason}`); else toast.error(`Run failed: ${result?.error || 'unknown error'}`); } catch (e) { diff --git a/client/src/pages/CreativeCommissionDetail.test.jsx b/client/src/pages/CreativeCommissionDetail.test.jsx index e119d70b1f..db8f3fa242 100644 --- a/client/src/pages/CreativeCommissionDetail.test.jsx +++ b/client/src/pages/CreativeCommissionDetail.test.jsx @@ -8,8 +8,8 @@ * degrades to the status-only card. */ -import { describe, it, expect, vi, beforeEach } from 'vitest'; -import { render, screen, waitFor } from '@testing-library/react'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { render, screen, waitFor, act, fireEvent } from '@testing-library/react'; import { MemoryRouter } from 'react-router'; vi.mock('../services/api', async (importOriginal) => ({ @@ -29,12 +29,14 @@ vi.mock('../components/ui/Toast', () => ({ default: { success: vi.fn(), error: vi.fn(), info: vi.fn(), warning: vi.fn() }, })); // ProjectPreview reaches into the media/job graph; the assertions here are about -// which projects resolved, so stub it down to an identifiable marker. +// which projects resolved (and, for #4149, which snapshot of a project is on +// screen), so stub it down to an identifiable marker carrying the status. vi.mock('../components/creative-director/ProjectPreview.jsx', () => ({ - default: ({ project }) =>
, + default: ({ project }) =>
{project.status}
, })); import * as api from '../services/api'; +import toast from '../components/ui/Toast'; import CreativeCommissionDetail from './CreativeCommissionDetail'; const COMMISSION = { @@ -105,3 +107,130 @@ describe('CreativeCommissionDetail render-history project resolution (#4148)', ( expect(screen.getByText('render unavailable')).toBeTruthy(); }); }); + +/** + * Live render refresh (#4149). + * + * A commission fire creates the CD project and returns; the render lands minutes + * later. The page used to sit on the stale "no render yet" card until a reload, + * and the Run-now toast said as much. It now polls the referenced projects while + * any `started` run still points at one that hasn't settled — and stops as soon + * as they all have, so an idle detail page issues no traffic. + */ +describe('CreativeCommissionDetail live render refresh (#4149)', () => { + // Drain pending promises (and optionally advance the poll clock) inside act. + const settle = async (ms = 0) => { await act(async () => { await vi.advanceTimersByTimeAsync(ms); }); }; + const mountLoaded = async () => { + render(); + // Commission fetch → id-set effect → project batch fetch: two awaits deep. + await settle(); + await settle(); + }; + const withRun = (ranAt) => ({ + ...COMMISSION, + runs: [{ id: 'run-1', projectId: 'cd-1', status: 'started', trigger: 'manual', ranAt }], + }); + + beforeEach(() => { + vi.clearAllMocks(); + api.getCommission.mockResolvedValue(COMMISSION); + api.getCreativeDirectorProjectsByIds.mockResolvedValue([]); + }); + afterEach(() => { vi.useRealTimers(); }); + + it('swaps a still-generating run to its finished render without a reload', async () => { + vi.useFakeTimers(); + api.getCommission.mockResolvedValue(withRun(new Date().toISOString())); + api.getCreativeDirectorProjectsByIds + .mockResolvedValueOnce([{ id: 'cd-1', status: 'rendering' }]) + .mockResolvedValue([{ id: 'cd-1', status: 'complete', finalVideoId: 'job-1' }]); + + await mountLoaded(); + expect(screen.getByTestId('preview-cd-1').textContent).toBe('rendering'); + const beforePoll = api.getCreativeDirectorProjectsByIds.mock.calls.length; + + await settle(5000); + expect(api.getCreativeDirectorProjectsByIds.mock.calls.length).toBeGreaterThan(beforePoll); + expect(screen.getByTestId('preview-cd-1').textContent).toBe('complete'); + }); + + it('stops polling once every referenced project has settled', async () => { + vi.useFakeTimers(); + api.getCommission.mockResolvedValue(withRun(new Date().toISOString())); + api.getCreativeDirectorProjectsByIds.mockResolvedValue([ + { id: 'cd-1', status: 'complete', finalVideoId: 'job-1' }, + ]); + + await mountLoaded(); + const settledCalls = api.getCreativeDirectorProjectsByIds.mock.calls.length; + + await settle(30000); + expect(api.getCreativeDirectorProjectsByIds.mock.calls.length).toBe(settledCalls); + }); + + it('never polls for a started run past the in-flight age ceiling', async () => { + vi.useFakeTimers(); + // A run whose project stalled mid-render hours ago: polling it forever would + // burn a request every 5s on any tab left open, and no poll can rescue it. + const stale = new Date(Date.now() - 7 * 60 * 60 * 1000).toISOString(); + api.getCommission.mockResolvedValue(withRun(stale)); + api.getCreativeDirectorProjectsByIds.mockResolvedValue([{ id: 'cd-1', status: 'rendering' }]); + + await mountLoaded(); + const initialCalls = api.getCreativeDirectorProjectsByIds.mock.calls.length; + + await settle(30000); + expect(api.getCreativeDirectorProjectsByIds.mock.calls.length).toBe(initialCalls); + }); + + it('treats a project the batch resolved as pruned as settled, not in flight', async () => { + vi.useFakeTimers(); + // The batch answered for this exact id set and omitted cd-1 — the project is + // gone. Polling can never resurrect it, so the page must not keep asking. + api.getCommission.mockResolvedValue(withRun(new Date().toISOString())); + api.getCreativeDirectorProjectsByIds.mockResolvedValue([]); + + await mountLoaded(); + expect(screen.getByText('render unavailable')).toBeTruthy(); + const prunedCalls = api.getCreativeDirectorProjectsByIds.mock.calls.length; + + await settle(30000); + expect(api.getCreativeDirectorProjectsByIds.mock.calls.length).toBe(prunedCalls); + }); + + it('keeps retrying while the project batch request is failing', async () => { + vi.useFakeTimers(); + // A FAILED fetch is not an authoritative "pruned" answer — an unresolved id + // still means "not known yet", so the poll has to stay armed. + api.getCommission.mockResolvedValue(withRun(new Date().toISOString())); + api.getCreativeDirectorProjectsByIds.mockRejectedValue(new Error('offline')); + + await mountLoaded(); + const failedCalls = api.getCreativeDirectorProjectsByIds.mock.calls.length; + + await settle(5000); + expect(api.getCreativeDirectorProjectsByIds.mock.calls.length).toBeGreaterThan(failedCalls); + }); + + it('refetches the render batch for a new run and drops the reload advice', async () => { + const started = { + id: 'run-new', projectId: 'cd-new', status: 'started', trigger: 'manual', + ranAt: new Date().toISOString(), + }; + api.getCommission.mockResolvedValue({ ...COMMISSION, runs: [] }); + api.runCommissionNow.mockResolvedValue({ + status: 'started', commission: { ...COMMISSION, runs: [started] }, + }); + render(); + await screen.findByRole('heading', { name: COMMISSION.name }); + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: /Run commission .* now/i })); + }); + + await waitFor(() => expect(api.getCreativeDirectorProjectsByIds) + .toHaveBeenCalledWith(['cd-new'], { silent: true })); + expect(toast.success).toHaveBeenCalledWith(expect.stringContaining('appears below')); + expect(toast.success).toHaveBeenCalledWith(expect.not.stringContaining('reload')); + }); +});