feat: persist Story song identity before generate - #140
Merged
Conversation
Mint a durable song-… candidate, save it pending on the Story library, then generate and patch the same id so a client close can recover the WAV.
PR Review — Loreframe StudioRisk: medium Automated review from Findings
Changed files
CONTRIBUTING checklist
Posted by the repo PR review workflow. Re-runs on each push to the PR. |
Code healthQuality score: 49.8/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.2 points.
Markdown, JSON catalogs and tests are out of this table. Only Most complex functions
Trend vs baseline
Warnings
Ratchet passed. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Recovered songs skip library autosave
- Recovery now caches the pre-recovery remote snapshot so the autosave subscriber sees a diff and PUTs the ready song.
- ✅ Fixed: Failed status blocks sidecar recovery
- Empty-source failed candidates are now treated as recoverable so a matching sidecar WAV can be reattached after a client timeout.
Or push these changes by commenting:
@cursor push ae76c6537a
Preview (ae76c6537a)
diff --git a/ui/src/features/stories/store.ts b/ui/src/features/stories/store.ts
--- a/ui/src/features/stories/store.ts
+++ b/ui/src/features/stories/store.ts
@@ -390,6 +390,11 @@
library.projects,
library.revision,
)
+ // Cache the pre-recovery snapshot (remote, or the just-uploaded first-run
+ // library) so sidecar recovery can queue the same PUT as a local-newer merge.
+ const remoteSerialized = remoteLibrary
+ ? JSON.stringify(remoteLibrary)
+ : JSON.stringify(library)
const recovered = conflicts.length
? { library, recovered: false }
: await recoverHydratedLibrary(workspace, library)
@@ -402,12 +407,9 @@
)
// A local-newer/exclusive merge must be sent back to the server. A
// conflict deliberately stays unsynced until a future explicit review.
- const remoteSerialized = remoteLibrary
- ? JSON.stringify(remoteLibrary)
- : JSON.stringify(library)
lastPersistedLibrary.set(
workspace,
- needsRemoteSync && !conflicts.length && !recovered.recovered
+ (needsRemoteSync || recovered.recovered) && !conflicts.length
? remoteSerialized
: JSON.stringify(library),
)
diff --git a/ui/src/features/stories/storySongRecovery.ts b/ui/src/features/stories/storySongRecovery.ts
--- a/ui/src/features/stories/storySongRecovery.ts
+++ b/ui/src/features/stories/storySongRecovery.ts
@@ -121,7 +121,9 @@
}
export function isPendingStoryMusicCandidate(candidate: StoryMusicCandidate): boolean {
- return candidate.status === 'pending' || (!candidate.source.trim() && candidate.status !== 'failed')
+ // Empty-source rows stay recoverable even after a client timeout marks them
+ // failed: the WAV and candidate_id sidecar may already be on disk.
+ return candidate.status === 'pending' || !candidate.source.trim()
}
export function libraryHasPendingSongs(library: Pick<StoryLibraryData, 'projects'>): boolean {
diff --git a/ui/tests/storySongRecovery.test.mjs b/ui/tests/storySongRecovery.test.mjs
--- a/ui/tests/storySongRecovery.test.mjs
+++ b/ui/tests/storySongRecovery.test.mjs
@@ -113,6 +113,61 @@
assert.equal(candidate.taskId, 'task-9')
})
+test('hydrate recovers a failed song when a matching sidecar already exists', async () => {
+ const { recoverPendingStorySongs } = await import('../src/features/stories/storySongRecovery.ts')
+ const { createStoryProject, normalizeStoryProject } = await import('../src/features/stories/model.ts')
+ const base = createStoryProject('music_video')
+ const failedId = 'song-timeout-failed'
+ const project = normalizeStoryProject({
+ ...base,
+ id: 'story-failed-recover',
+ music: {
+ ...base.music,
+ cues: [{
+ id: 'cue-failed',
+ kind: 'story',
+ targetId: 'story-failed-recover',
+ title: 'Failed',
+ purpose: '',
+ referenceSong: '',
+ brief: '',
+ style: 'metal',
+ lyrics: '[Verse]\nFailed',
+ lyriaPrompt: '',
+ instrumental: false,
+ durationSeconds: 30,
+ candidates: [{
+ id: failedId,
+ status: 'failed',
+ name: '',
+ source: '',
+ prompt: 'metal',
+ lyrics: '[Verse]\nFailed',
+ provider: 'local',
+ model: 'ace_step_v1_5_xl_sft_lm_4b',
+ durationSeconds: 30,
+ createdAt: '2026-09-05T00:00:00.000Z',
+ }],
+ }],
+ },
+ })
+ const recovered = recoverPendingStorySongs({ [project.id]: project }, [{
+ candidateId: failedId,
+ filename: 'timeout.wav',
+ source: '/api/v1/file/timeout.wav',
+ projectId: project.id,
+ cueId: 'cue-failed',
+ taskId: 'task-timeout',
+ }])
+ assert.equal(recovered.changed, true)
+ const candidate = recovered.projects[project.id].music.cues[0].candidates[0]
+ assert.equal(candidate.id, failedId)
+ assert.equal(candidate.status, 'ready')
+ assert.equal(candidate.name, 'timeout.wav')
+ assert.equal(candidate.source, '/api/v1/file/timeout.wav')
+ assert.equal(candidate.taskId, 'task-timeout')
+})
+
test('recovery ignores a sidecar from another project or workspace candidate', async () => {
const { recoverPendingStorySongs } = await import('../src/features/stories/storySongRecovery.ts')
const { createStoryProject, normalizeStoryProject } = await import('../src/features/stories/model.ts')
@@ -215,15 +270,24 @@
libraryConflicts: [],
})
const originalFetch = globalThis.fetch
+ const putBodies = []
t.after(() => {
globalThis.fetch = originalFetch
window.localStorage.removeItem(`maestro-story-library-v2:${workspace}`)
})
- globalThis.fetch = async input => {
+ globalThis.fetch = async (input, init = {}) => {
const url = String(input)
if (url.includes('/api/v1/stories/library?')) {
return new Response(JSON.stringify(library), { headers: { 'content-type': 'application/json' } })
}
+ if (url.endsWith('/api/v1/stories/library') && init.method === 'PUT') {
+ const body = JSON.parse(String(init.body || '{}'))
+ putBodies.push(body.library)
+ return new Response(JSON.stringify({
+ ...body.library,
+ revision: (body.baseRevision || library.revision) + 1,
+ }), { headers: { 'content-type': 'application/json' } })
+ }
if (url.includes('/api/v1/assets')) {
return new Response(JSON.stringify({
assets: [{
@@ -263,4 +327,12 @@
assert.equal(recovered.status, 'ready')
assert.equal(recovered.name, 'closed.wav')
assert.match(recovered.source, /closed\.wav/)
+
+ const deadline = Date.now() + 2000
+ while (!putBodies.length && Date.now() < deadline) {
+ await new Promise(resolve => setTimeout(resolve, 50))
+ }
+ assert.equal(putBodies.length > 0, true)
+ assert.equal(putBodies[0].projects[project.id].music.cues[0].candidates[0].status, 'ready')
+ assert.match(putBodies[0].projects[project.id].music.cues[0].candidates[0].source, /closed\.wav/)
})You can send follow-ups to the cloud agent here.
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 5d51353. Configure here.
| projects: library.projects, | ||
| libraryRevision: library.revision, | ||
| dirty: false, | ||
| dirty: needsRemoteSync || recovered.recovered, |
There was a problem hiding this comment.
Recovered songs skip library autosave
Medium Severity
After loadWorkspace reattaches a pending song, lastPersistedLibrary is set to the recovered snapshot. The autosave subscriber only writes when that cache differs from the current library, so the ready patch never reaches the server. dirty is set but is not what triggers persist. Another tab or device still sees pending.
Reviewed by Cursor Bugbot for commit 5d51353. Configure here.
Owner
Author
There was a problem hiding this comment.
Addressed in dc63eb5: recovery now PUTs the ready library immediately. lastPersistedLibrary is only updated after that save succeeds; a failed save keeps the pre-recovery remote snapshot so autosave can retry.
|
|
||
| export function isPendingStoryMusicCandidate(candidate: StoryMusicCandidate): boolean { | ||
| return candidate.status === 'pending' || (!candidate.source.trim() && candidate.status !== 'failed') | ||
| } |
There was a problem hiding this comment.
Failed status blocks sidecar recovery
Medium Severity
Any generate error calls markSongFailed, and recovery only reattaches rows that still look pending. A client timeout after the WAV and candidate_id sidecar are already on disk leaves a durable failed row. Later loadWorkspace will not bind that audio, and videoclip staging refuses the version.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 5d51353. Configure here.
Owner
Author
There was a problem hiding this comment.
Addressed in dc63eb5: isRecoverableStoryMusicCandidate includes failed rows without a ready source. loadWorkspace reattaches them when the sidecar has the same candidate_id.
Autosave skipped the ready patch because lastPersistedLibrary already held the recovered snapshot. Write that patch to the Story library immediately. Also recover failed rows when a matching sidecar exists after a client timeout.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resumen ejecutivo
Qué cambia
Una versión de canción de Story Lab ahora tiene identidad antes de generar
el audio. El ID durable es
song-….v1/v2/v3solo es una etiqueta depantalla. El candidato se guarda como
pendingen la biblioteca de Story endisco antes de llamar a generate; si el cliente se cierra, el WAV se puede
volver a enganchar por
candidate_iddel sidecar.Para qué sirve
Hoy el Wizard mintaba el ID, generaba y después guardaba. Si se cerraba el
cliente, el WAV quedaba huérfano. Story Lab UI mintaba el ID después de
generar y no enviaba procedencia.
normalizetiraba candidatos sinsource,así que un pending no podía persistir. Este PR corta esa pérdida sin tocar
Launch,
useStoreniagentActions.Impacto para el usuario
Al generar una canción desde el Wizard o Story Lab → Music, la versión queda
registrada enseguida. Si se cierra la pestaña a mitad de generate, al reabrir
el workspace la canción vuelve a aparecer cuando el WAV ya está en disco. Un
videoclip solo se puede preparar con una versión lista, no con una pendiente
o fallida.
Riesgo
Estado
Summary
Makes Story song identity survive client close by saving a pending
StoryMusicCandidate(song-…) to the Story library with CAS beforecompute starts, then patching that same id to
readyorfailed. Displayv1/v2is denormalized and is never identity.Does not touch
_launch_runtime.py,ui/src/stores/useStore.tsorui/src/features/agent/agentActions.ts. Server-sidegenerate-musicattachis a follow-up PR. This slice is not done until required CI and
Cursor/Bugbot complete.
Overview
targetStoryId/ current project), then unique title.cueId. Title only if no ID; a stale title fails.candidateId = storyId('song')and displayversionbefore generate.status: 'pending'with empty source and provenance{ projectId, cueId, candidateId, songVersion }via library CAS.{ actor, capability, project_id, cue_id, candidate_id, song_version }.source, filename, task/job ids,ready).failedand keep the id for retry lineage.Refuse synthetic cue
story-songunless the caller passed that exact id.loadWorkspacerecovers pending rows whose sidecar/output has matchingcandidate_id.normalizeMusicCandidatekeeps pending/failed rows with a stable id andnever remints an existing empty id on load.
Wizard
generateStorySongand Story LabgenerateMusicCueAudio(ACE-Step,MiniMax Music 3 local, MiniMax remote) share
generateStoryCueSong.Detailed changes
Backend
attach_story_song_candidate(...)inapp/services/story_library.pyCAS-patchesone pending row by project/cue/candidate IDs. Isolated per workspace directory.
_launch_runtime.py.UI and Wizard
ui/src/features/stories/storySongGeneration.tsshared helper.ui/src/features/stories/storySongRecovery.tssidecar/output matching.generateMusicCueAudiocalls the helper instead of minting after generate.pinokio.js/ launchers untouched. Story Lab panel shrinks (~80 lines extracted).Data, provenance and compatibility
StoryMusicCandidate.status?: 'pending' | 'ready' | 'failed'.ready.story-songis no longer a silent staging fallback.Files and ownership
ui/src/features/stories/types.tsstatusonStoryMusicCandidateui/src/features/stories/model.tsui/src/features/stories/musicWorkflowState.tsui/src/features/stories/storySongGeneration.tsui/src/features/stories/storySongRecovery.tsui/src/features/stories/actions.tsui/src/features/stories/StoryLabPanel.tsxgenerateMusicCueAudiocalls the helperui/src/features/stories/store.tsloadWorkspaceui/src/features/stories/musicVideoSelection.tsui/src/features/stories/storyProductionController.tsstory-songapp/services/story_library.pyattach_story_song_candidatefor recovery/testsdocs/development/STORY_SONG_IDENTITY.mdDeliberately not edited:
_launch_runtime.py,ui/src/stores/useStore.ts,ui/src/features/agent/agentActions.ts.SLICE_QUEUE.mdleft alone because#136 is still open.
Validation
python scripts/verify_clean_repo.py— not run as a standalone step;validate_local.shis the mandated gatepytest -q tests/test_story_library.py→ 10 passedstorySongRevision,storySongRecovery,musicVideoSelection,storyProductionController,agentActions→ 95 passedcd ui && npm test→ 636 passedcd ui && npm run i18n:check— covered by lint/build gate (no catalog keys added)cd ui && npm run lint -- --max-warnings=0→ passcd ui && npm run build→ passgit diff --check→ passnpm run test:e2e→ 7 passed (simulated, no GPU)PYTHON=... bash scripts/validate_local.sh→ completePYTHON=... bash scripts/check_code_health_pr_base.sh→ Ratchet passedNo real MiniMax/ACE-Step generation tests.
Code quality
9ac4cacbui/src/features/stories/store.ts22 → 25CI will republish the table; do not treat this as certified until CI comments.
CI and review
This PR is not done while required CI or Cursor/Bugbot is still running.
Coste de la tarea
Notes and limitations
loadWorkspacematches pending rows to asset catalog /output metadata
candidate_id. Server-side attach insidegenerate-musicisexplicitly deferred.
sends them so a later PR can persist them without changing this contract.
story-songremains available only when the caller passes thatexact id (legacy global candidates).
Follow-up work
attach_story_song_candidateintogenerate-musicin_launch_runtime.py(sequential PR; that file is reserved).
autosave + next hydrate.
SLICE_QUEUE.md).Checklist
marked as waiting for them.
Note
Medium Risk
Changes Story library persistence, generation ordering, and videoclip staging gates across Wizard, Story Lab, and normalize/load paths; backend attach is untested in production HTTP until a follow-up.
Overview
Story Lab song versions now get a durable
song-…id before audio generation. The PR saves a pendingStoryMusicCandidateto the Story library (CAS) with emptysourceand full provenance, then calls generate withproject_id,cue_id,candidate_id, andsong_version. Success patches the same row toready; failure marksfailedwithout minting a new id.Wizard
generateStorySongand Story Lab cue audio generation are unified ingenerateStoryCueSong.loadWorkspacecan reattach finished WAVs to pending rows by matching asset/sidecarcandidate_id. Normalization keeps pending/failed rows with stable ids and no longer drops or remints empty-source candidates.Videoclip staging and music-video selection now require a ready candidate and tighten the legacy
story-songsynthetic cue (only when explicitly requested). Backend addsattach_story_song_candidatefor CAS patch-by-id (tests only; no route yet). Docs addSTORY_SONG_IDENTITY.mdand domain-model notes.Reviewed by Cursor Bugbot for commit 5d51353. Configure here.