feat: add portable generation-record v1 contract - #138
Conversation
Project asset-manifest, provenance and job lifecycle into one typed attempt record with durable JSON persistence, cancellation and resume. Retries mint a new generation_id and keep asset_id only for the same artifact. Launch/Activity/Library wiring is left to a later PR.
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.5/100Higher is better. The score is a trend dashboard; the independent ratchet below remains the CI gate.
Change vs PR base: +0.3 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 5 potential issues.
Bugbot Autofix prepared fixes for all 5 issues found in the latest run.
- ✅ Fixed: Cancel loses to multi-phase requeue
- transition_status now settles a requested cancel on queued and running targets as well, so multi-phase requeue/start cannot continue after cancel.
- ✅ Fixed: UI cancel overwrites finished records
- applyCancel now leaves cancelled records unchanged and throws on completed/failed, matching the Python apply_cancel contract.
- ✅ Fixed: Unknown manifest status becomes planned
- mapAssetManifestStatus now maps unrecognized non-empty statuses to failed with invalid_status, same as the Python mapper.
- ✅ Fixed: Retry helper reuses the same IDs
- retryGeneration now mints unique generation_id and asset_id values with crypto.randomUUID so repeated retries no longer collide.
- ✅ Fixed: Patch writes the wrong duration field
- toAssetManifestPatch now writes timestamps.duration_ms as timing.total_ms so a later Python projection can recover duration.
Or push these changes by commenting:
@cursor push e134a22599
Preview (e134a22599)
diff --git a/app/services/generation_record.py b/app/services/generation_record.py
--- a/app/services/generation_record.py
+++ b/app/services/generation_record.py
@@ -814,7 +814,9 @@
"""Apply one legal lifecycle transition. Cancellation already requested wins."""
current = validate_generation_record(record)
resolved = (_clean(target) or "").casefold()
- if current["cancellation"]["requested"] and resolved in {"completed", "failed"}:
+ if current["cancellation"]["requested"] and resolved in {
+ "queued", "running", "completed", "failed",
+ }:
return apply_cancel(current, reason=current["cancellation"].get("reason"), at=at)
if not is_legal_transition(current["status"], resolved):
raise GenerationRecordError(
diff --git a/ui/src/lib/generationRecord.ts b/ui/src/lib/generationRecord.ts
--- a/ui/src/lib/generationRecord.ts
+++ b/ui/src/lib/generationRecord.ts
@@ -213,7 +213,14 @@
if (STATUS_SET.has(raw)) {
return { status: raw as GenerationStatus, resultKind: null, error: null }
}
- return { status: 'planned', resultKind: null, error: null }
+ if (!raw) {
+ return { status: 'planned', resultKind: null, error: null }
+ }
+ return {
+ status: 'failed',
+ resultKind: null,
+ error: { code: 'invalid_status', message: `Unsupported status '${raw}'` },
+ }
}
export function mapGenerationStatusToManifest(
@@ -370,7 +377,13 @@
parameters: record.model.configuration,
inputs: parents,
},
- timing: record.timestamps,
+ timing: {
+ created_at: record.timestamps.created_at,
+ queued_at: record.timestamps.queued_at,
+ started_at: record.timestamps.started_at,
+ completed_at: record.timestamps.completed_at,
+ total_ms: record.timestamps.duration_ms,
+ },
lineage: { parents, transformations: [] },
technical: { generation_id: record.generation_id, result: record.result },
}
@@ -381,8 +394,8 @@
'asset_id' | 'generation_id' | 'retry_count' | 'lineage' | 'status' | 'workspace_id'
> {
return {
- generation_id: `gen_retry_${record.generation_id}`,
- asset_id: sameArtifact ? record.asset_id : `asset_retry_${record.asset_id}`,
+ generation_id: `gen_${crypto.randomUUID().replace(/-/g, '')}`,
+ asset_id: sameArtifact ? record.asset_id : `asset_${crypto.randomUUID().replace(/-/g, '')}`,
retry_count: record.retry_count + 1,
status: 'planned',
workspace_id: record.workspace_id,
@@ -409,6 +422,12 @@
}
export function applyCancel(record: GenerationRecord): Pick<GenerationRecord, 'status' | 'cancellation'> {
+ if (record.status === 'cancelled') {
+ return { status: record.status, cancellation: record.cancellation }
+ }
+ if (record.status === 'completed' || record.status === 'failed') {
+ throw new Error('Cannot cancel a finished generation')
+ }
return {
status: 'cancelled',
cancellation: { requested: true, at: record.cancellation.at, reason: record.cancellation.reason },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 4aba32c. Configure here.
| if not is_legal_transition(current["status"], resolved): | ||
| raise GenerationRecordError( | ||
| f"Illegal generation transition {current['status']!r} -> {resolved!r}", | ||
| ) |
There was a problem hiding this comment.
Cancel loses to multi-phase requeue
Medium Severity
transition_status only turns a requested cancel into cancelled when the target is completed or failed. A running record with cancellation.requested can still move to queued and back to running, so multi-phase work continues after the user cancelled.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4aba32c. Configure here.
There was a problem hiding this comment.
Fixed in b66ea0a. transition_status now applies cancel for any non-cancelled target once cancellation.requested is set, including queued/running requeue. Covered by test_cancellation_before_and_during_running.
| status: 'cancelled', | ||
| cancellation: { requested: true, at: record.cancellation.at, reason: record.cancellation.reason }, | ||
| } | ||
| } |
There was a problem hiding this comment.
UI cancel overwrites finished records
Medium Severity
applyCancel always returns cancelled, including when the record is already completed or failed. The Python helper rejects that, and the contract says terminal states do not transition, so the UI mapper can relabel finished work as cancelled.
Reviewed by Cursor Bugbot for commit 4aba32c. Configure here.
There was a problem hiding this comment.
Fixed in b66ea0a. applyCancel now leaves completed, failed, and cancelled records unchanged, matching the Python helper.
| return { status: raw as GenerationStatus, resultKind: null, error: null } | ||
| } | ||
| return { status: 'planned', resultKind: null, error: null } | ||
| } |
There was a problem hiding this comment.
Unknown manifest status becomes planned
Medium Severity
mapAssetManifestStatus maps any unrecognized execution.status to planned with no error. The Python mapper treats the same input as failed with invalid_status, so the UI can present a corrupt or unsupported sidecar as a generation that has not started.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4aba32c. Configure here.
There was a problem hiding this comment.
Fixed in b66ea0a. Unrecognized manifest statuses now map to failed with invalid_status, same as Python map_manifest_status.
| derivatives: [], | ||
| }, | ||
| } | ||
| } |
There was a problem hiding this comment.
Retry helper reuses the same IDs
Medium Severity
retryGeneration builds generation_id as gen_retry_${parent} and a new asset_id as asset_retry_${parent}. A second retry of the same parent mints the same IDs, which breaks policy (b) that every attempt gets a new generation_id.
Reviewed by Cursor Bugbot for commit 4aba32c. Configure here.
There was a problem hiding this comment.
Fixed in b66ea0a. retryGeneration mints unique IDs via crypto.randomUUID (with a timestamp fallback) instead of gen_retry_${parent}.
| timing: record.timestamps, | ||
| lineage: { parents, transformations: [] }, | ||
| technical: { generation_id: record.generation_id, result: record.result }, | ||
| } |
There was a problem hiding this comment.
Patch writes the wrong duration field
Medium Severity
toAssetManifestPatch copies record.timestamps onto timing, which keeps duration_ms. Asset-manifest timing uses total_ms, and the Python patch maps that field correctly. Applying the UI patch therefore drops duration for any later Python projection.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 4aba32c. Configure here.
There was a problem hiding this comment.
Fixed in b66ea0a. toAssetManifestPatch now writes timing.total_ms from timestamps.duration_ms.
A requested cancel now wins over requeue. UI cancel leaves completed/failed records alone, retries mint unique IDs, unknown manifest statuses fail, and the asset-manifest patch writes total_ms.
1 participant
Resumen ejecutivo
Qué cambia
Se añade un contrato portable de generación v1: un documento JSON tipado
que describe un intento de generar cualquier elemento (imagen, audio, vídeo,
3D, etc.). No sustituye los ficheros generados ni el manifiesto de asset
existente; lee y escribe sobre esas fuentes.
Para qué sirve
Hoy el estado de una generación está repartido entre el manifiesto de asset,
la cola durable, el ciclo de vida del job y la procedencia. Este contrato da
una identidad estable (
generation_id/asset_id), estados legales,cancelación, reanudación tras un reinicio y aislamiento por Workspace, sin
inventar un segundo almacén de medios.
Impacto para el usuario
Ninguno en esta PR. No hay cambios de UI visible ni de generación real. El
cableado a Launch, Activity y Library queda para un PR posterior porque
_launch_runtime.pyestá reservado por #135.Riesgo
Estado
Summary
Adds GenerationRecord v1 as a portable read/write projection over the existing
asset-manifest v1 sidecar, generation provenance and job lifecycle. Persistence
uses atomic temp-file replace (the DurableGenerationQueue pattern). Public
status is the six-value enum
planned | queued | running | completed | failed | cancelled. Asset-manifestpreparedmaps toplanned;partialis not aseventh status (
completed+result.kind = "partial"when a filename exists,otherwise
failed).Attempt identity follows policy (b): a retry mints a new
generation_id,increments
retry_counton the new attempt, and links the parent inlineage.parents.asset_idis reused only when the bytes are the sameartifact. Resume after a simulated process restart reloads the JSON and keeps
the last durable status; it never invents success.
The module does not import FastAPI, WanGP or launch. Titles/prompts are not
identity. Secrets in model configuration are redacted.
workspace_idis acollection ID and
output_folder/ location filenames are never host paths.A record saved under workspace A cannot be listed or loaded as workspace B.
Overview
build_generation_recordcreates a canonical attempt.project_from_asset_manifestand
to_asset_manifest_patchkeep the two contracts compatible.GenerationRecordStorepersists one JSON file per attempt under{root}/{workspace_id}/{generation_id}.json.transition_statusenforces legaltransitions (including
running -> queuedfor multi-phase work).request_cancelsettles planned/queued immediately and leaves running recordsin
runningwithcancellation.requesteduntilapply_cancel, matchingjob-lifecycle acknowledgement.
UI
ui/src/lib/generationRecord.tsmirrors the types and pure helpers. Nostore, Activity or launch wiring.
Detailed changes
Backend
app/services/generation_record.py: contract, projection, lifecycle,atomic persist/load/list, cross-workspace isolation, retry lineage.
_launch_runtime.pyedits.UI and Wizard
ui/src/lib/generationRecord.tstypes and pure helpers.ui/tests/generationRecord.test.ts.Data, provenance and compatibility
docs/development/generation-record-v1.schema.json.docs/development/GENERATION_RECORD.md.DOMAIN_MODEL_AND_ASSET_PROVENANCE.md.Files and ownership
app/services/generation_record.pydocs/development/generation-record-v1.schema.jsondocs/development/GENERATION_RECORD.mddocs/development/DOMAIN_MODEL_AND_ASSET_PROVENANCE.mdtests/test_generation_record.pyui/src/lib/generationRecord.tsui/tests/generationRecord.test.tsDeliberately untouched (owned by #135 or out of scope):
app/_launch_runtime.py,ui/src/stores/useStore.ts,ui/src/features/agent/agentActions.ts,ui/src/features/stories/StoryLabPanel.tsx,ui/src/features/stories/musicModel.ts,ui/src/features/agent/studioCapabilities.ts,app/routers/llm.py,scripts/code_health_baseline.json.Validation
python scripts/verify_clean_repo.py— PASSpython -m compileall -q app/services/generation_record.py— PASSpython -m pytest -q tests/test_generation_record.py— 14 passedpython scripts/architecture_contracts.pyandpytest -q tests/test_architecture_contracts.py— PASS (new test does not read launch/useStore)cd ui && npx tsx --tsconfig tsconfig.app.json --import ./tests/setupI18n.ts --test tests/generationRecord.test.ts— 7 passedcd ui && npm test && npm run lint -- --max-warnings=0 && npm run build— PASS viavalidate_local.shgit diff --check— PASSbash scripts/validate_local.shsimulated Playwright — 7 passed; no GPU/real mediascripts/check_code_health_pr_base.sh(that script is not in the tree):python scripts/code_health.py --check --baseline <origin/main report>— PASS. Production LOC +1,493 (budget ~2,000+); functions ≥15 +0; max complexity +0; new filegeneration_record.pyis 1,077 lines (limit 1,200). WARN: production LOC increased by +1,493.PYTHON=/home/ina/pinokio/api/Maestro-next.git/app/env/bin/python bash scripts/validate_local.shcompleted with[local] complete (no GPU or external provider calls).Code quality
main: pending CILocal ratchet versus
origin/mainpassed as recorded above. The publishedquality score remains the CI comment; this PR does not invent it.
CI and review
Do not treat this PR as done while required CI or review is still running.
Coste de la tarea
Notes and limitations
Studio/Director yet.
weights (same limitation as
DurableGenerationQueue).scripts/check_code_health_pr_base.shis not present on this branch; thesame comparison is implemented inside
scripts/validate_local.sh.Follow-up work
_launch_runtime.py, Activity and the Librarycatalog in a later sequential PR.
#135currently owns_launch_runtime.py.persist
technical.generation_idon the sidecar viato_asset_manifest_patch.Checklist
marked as waiting for them.
Note
Low Risk
Additive library, schema, and docs only; no API routes, launch runtime, or user-facing behavior changes until a follow-up wires writers.
Overview
Introduces Generation record v1 as a typed JSON contract for a single asset-generation attempt, layered on asset-manifest v1, provenance, and job lifecycle—without a second media store or Launch/Activity wiring in this PR.
Backend (
generation_record.py) adds build/validate, six public statuses (plannedthroughcancelled) with manifest mapping (prepared→planned;partialviaresult.kind), legal transitions, cancel/retry/resume (policy b: newgeneration_idper retry with parent lineage), bidirectionalproject_from_asset_manifest/to_asset_manifest_patch, secret redaction and portable paths, plusGenerationRecordStorewith atomic persist and workspace-scoped load/list.Docs add
GENERATION_RECORD.md,generation-record-v1.schema.json, and a Generation noun in the domain model.UI mirrors the contract in
ui/src/lib/generationRecord.tswith matching pure helpers and tests. Python and UI tests cover schema, persistence, isolation, cancellation, and manifest projection.Reviewed by Cursor Bugbot for commit 4aba32c. Configure here.