fix: generation-record authority as optional-workspace projection with CAS - #142
Merged
Conversation
…h CAS Keep GenerationRecord as a projection, not a second store. workspace_id is an optional collection (never copied from output_folder). Merge does not wipe lineage with empty lists. Resume of running/queued marks interrupted reconciliation instead of inventing success. Persist rejects stale revisions.
Owner
Author
|
cursor review |
PR Review — Loreframe StudioRisk: low 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.0 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 3 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for all 3 issues found in the latest run.
- ✅ Fixed: Patch drops transformation artifact ids
- to_asset_manifest_patch and toAssetManifestPatch now map transformations through the same artifact-ref helper as parents, so write-back keeps id instead of asset_id.
- ✅ Fixed: Duration fallback uses inference span
- _timestamps_block now derives missing duration_ms from created_at to completed_at, matching _stamp_transition and asset-manifest total_ms wall time.
- ✅ Fixed: TS merge desyncs prompt fields
- mergeGenerationRecord now promotes prompt_full into prompt_effective and rebuilds prompt_display, matching Python merge_generation_record.
Or push these changes by commenting:
@cursor push 0044c4dca3
Preview (0044c4dca3)
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
@@ -380,7 +380,7 @@
queue_ms = _optional_ms(raw.get("queue_ms"))
inference_ms = _optional_ms(raw.get("inference_ms"))
if duration is None:
- duration = _milliseconds(started_at or created_at, completed_at)
+ duration = _milliseconds(created_at, completed_at)
if queue_ms is None:
queue_ms = _milliseconds(queued_at or created_at, started_at)
if inference_ms is None:
@@ -927,8 +927,11 @@
if parents:
lineage_patch["parents"] = parents
transformations = [
- dict(item) for item in value["lineage"].get("transformations") or []
- if isinstance(item, Mapping) and item
+ item for item in (
+ _artifact_parent(raw)
+ for raw in value["lineage"].get("transformations") or []
+ if isinstance(raw, Mapping)
+ ) if item
]
if transformations:
lineage_patch["transformations"] = transformations
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
@@ -281,6 +281,11 @@
return value.map(lineageRef).filter((item): item is GenerationLineageRef => item != null)
}
+function artifactRef(item: GenerationLineageRef): { id: string; kind: string; uri?: string } | null {
+ if (!item.asset_id) return null
+ return { id: item.asset_id, kind: item.kind || 'other', ...(item.uri ? { uri: item.uri } : {}) }
+}
+
function manifestPromptPair(prompts: JsonMap): { original: string; effective: string } {
const original = firstText(prompts.original) || ''
const effective = firstText(prompts.effective) || ''
@@ -382,9 +387,10 @@
export function toAssetManifestPatch(record: GenerationRecord): JsonMap {
const filename = record.location.filename
- const parents = record.lineage.parents.flatMap(item => (
- item.asset_id ? [{ id: item.asset_id, kind: item.kind || 'other', ...(item.uri ? { uri: item.uri } : {}) }] : []
- ))
+ const parents = record.lineage.parents.flatMap(item => {
+ const ref = artifactRef(item)
+ return ref ? [ref] : []
+ })
const patch: JsonMap = {
asset: { id: record.asset_id, filename, uri: record.location.uri || filename },
origin: {
@@ -421,7 +427,10 @@
},
technical: { generation_id: record.generation_id, result: record.result },
}
- const transformations = record.lineage.transformations || []
+ const transformations = (record.lineage.transformations || []).flatMap(item => {
+ const ref = artifactRef(item)
+ return ref ? [ref] : []
+ })
const lineage: JsonMap = {}
if (parents.length) lineage.parents = parents
if (transformations.length) lineage.transformations = transformations
@@ -436,11 +445,30 @@
: null
const next: GenerationRecord = { ...base }
for (const [key, value] of Object.entries(incoming)) {
- if (key === 'generation_id' || key === 'asset_id' || key === 'workspace_id' || key === 'schema' || key === 'schema_version' || key === 'lineage') {
+ if (
+ key === 'generation_id' || key === 'asset_id' || key === 'workspace_id'
+ || key === 'schema' || key === 'schema_version' || key === 'lineage'
+ || key === 'prompt_full' || key === 'prompt_original'
+ || key === 'prompt_effective' || key === 'prompt_display'
+ ) {
continue
}
;(next as unknown as JsonMap)[key] = value
}
+ if (incoming.prompt_original != null) {
+ next.prompt_original = String(incoming.prompt_original)
+ }
+ if (incoming.prompt_effective != null) {
+ next.prompt_effective = String(incoming.prompt_effective)
+ } else if (incoming.prompt_full != null) {
+ next.prompt_effective = String(incoming.prompt_full)
+ }
+ if (incoming.prompt_full != null) {
+ next.prompt_full = String(incoming.prompt_full)
+ }
+ const display = next.prompt_effective || next.prompt_original || next.prompt_full
+ next.prompt_full = display
+ next.prompt_display = truncatePromptDisplay(display)
if (lineagePatch) {
const lineage = {
parents: [...base.lineage.parents],You can send follow-ups to the cloud agent here.
Map transformations to asset-manifest artifact refs (id), derive duration_ms from created_at as total wall time, and keep TS prompt_full/effective/display in sync on merge.
Owner
Author
|
cursor review |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit a6adbbe. Configure here.
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.
Problem
Generation-record v1 (#138) still treated
workspace_idas a required collection (and could copyoutput_folderinto it), merged empty lineage lists as wipes, treated a re-read ofrunningas a live worker, and persisted with only an in-process RLock + atomic rename. That is not enough for two writers, and it is not a projection contract.Final behavior
AUTHORITY = "projection"). Bytes stay in asset-manifest v1; this JSON is a durable attempt projection with CAS, not a second catalog or scheduler.run_idis a correlation, not a new Run store).workspace_idis an optional collection (string | null).output_folderis the physical folder. Unscoped records are not members of a collection named like the folder.merge_generation_record/mergeGenerationRecord: empty or missing lineage lists do not wipe existing parents/derivatives/transformations. Identity cannot change.to_asset_manifest_patchomits empty lineage arrays and preservesprompt_original/prompt_effectiveplusqueue_ms/inference_ms/total_ms.resumeofqueued/runningwithoutworker_alive=Truesetsreconciliation.needed+interruptedand never inventscompleted.revisionand rejects stale expected revisions. Load does not mutate the file.Scope
app/services/generation_record.py(contract)app/services/generation_record_io.py(CAS persist, merge, resume, store)ui/src/lib/generationRecord.ts+ testsdocs/development/GENERATION_RECORD.md,fase3.mdNot in this PR:
_launch_runtime.py,useStore.ts,agentActions.ts, StoryLabPanel, producer wiring, file moves (F3.9).Tests
pytest tests/test_generation_record.py— 18 passed (merge, CAS two writers, unscoped store, original/effective prompts, resume interrupted)pytest tests/test_architecture_contracts.pygenerationRecord.test.ts— 10 passedbash scripts/validate_local.sh— OK (E2E 7/7)bash scripts/check_code_health_pr_base.shvsd4263ce6— passed, +38 hotspot lines (budget 75), score 49.8 +0.0Risks / limits
_physical/<output_folder>/in the JSON store. That bucket name is a store detail, not a workspace id.Base
origin/maind4263ce6. Independent of #141 (lyrics). Do not merge automatically. Fase 4 waits for this merge.Note
Medium Risk
Contract and persistence semantics change (nullable workspace, CAS revisions, merge rules); impact is limited today because only tests call the store, but future writers must adopt the new revision and resume behavior.
Overview
This PR reframes generation-record v1 as a projection over asset-manifest (
AUTHORITY = "projection"), splits contract (generation_record.py) from CAS persistence (generation_record_io.py), and tightens the schema so writers cannot treat the JSON store as a second catalog.Workspace & location:
workspace_idis now optional (null= unscoped);output_folderis required and is never copied intoworkspace_id. Unscoped records persist under_physical/<output_folder>/, and load/list enforce exact collection membership (includingnull).Richer metadata & patches: Records carry separate
prompt_original/prompt_effective,lineage.transformations,timestamps.queue_ms/inference_ms,revision, andreconciliation. Manifest projection andto_asset_manifest_patchround-trip these fields and omit empty lineage (andorigin.workspace_idwhen unscoped) so patches do not wipe sidecar lineage.Concurrency & recovery:
persist_generation_recordincrementsrevisionand rejects stale writers;merge_generation_recordunions non-empty lineage without wiping on empty lists.resume(defaultworker_alive=False) setsreconciliation.needed+interruptedfor queued/running without inventingcompleted.Python, JSON schema,
GENERATION_RECORD.md, andui/src/lib/generationRecord.ts(+ tests) stay aligned. No launch/runtime producer wiring in this PR.Reviewed by Cursor Bugbot for commit a6adbbe. Configure here.