Follow-up from #557, which made source.ifc publication locally failure-atomic and left one window open deliberately rather than half-closing it.
The residual
authoring_shared.publish_source_ifc promotes a staged file, publishes the same bytes to object storage, sets Project.source_ifc, and commits — all inside one pid_lock.mutating(pid) interval. On failure it restores the previous local file. That covers the local path, which is what bake_layers, /edit and the converter actually open.
It does not cover this ordering:
os.replace(staged, final) — succeedsstorage.put_stream(...) — succeedsdb.commit() — fails
Local file and row are rolled back to the previous model; object storage keeps the newer bytes. The helper's docstring states this explicitly rather than leaving it implied.
Why it is bounded today
- Storage is the durable copy, not what readers open — no request path reads model geometry out of object storage.
- The next successful publish overwrites the key, so the divergence does not accumulate.
- It needs a commit failure after a successful storage write, which is the narrow end of an already narrow window.
It is still real, and the fix is a design change rather than a guard.
What closing it properly requires
Publish each model as an immutable versioned artifact and move the pointer last, so nothing is ever overwritten in place and a failure at any step leaves the previous version wholly intact:
- a versioned storage key (
<project>/models/<version>/source.ifc) instead of the fixed <project>/source.ifc;
Project.source_ifc (or a successor column) naming a version, with the local path derived from it;- a retention policy — this is the part that needs a maintainer decision. Keeping every version is unbounded growth on a format where a single model is routinely hundreds of MB; keeping N is a number somebody has to choose; keeping only the current one reintroduces the overwrite this exists to remove.
Blast radius (why it was not folded into #557)
Four things assume a fixed path or key today:
PR #557 was four review rounds deep on a different defect. Landing a half-version of this inside it would have left two versioning schemes and a fixed key, which is worse than the single named window.
Done looks like
- a publish that fails at any step leaves every prior artifact — local, storage, row — byte-for-byte intact;
test_ifc_publish_atomic gains a check for the storage arm equivalent to the local one it has now (patch db.commit to raise after a successful put_stream, assert storage still serves the old bytes);- a written retention policy, enforced in code rather than recorded in prose.
Follow-up from #557, which made
source.ifcpublication locally failure-atomic and left one window open deliberately rather than half-closing it.The residual
authoring_shared.publish_source_ifcpromotes a staged file, publishes the same bytes to object storage, setsProject.source_ifc, and commits — all inside onepid_lock.mutating(pid)interval. On failure it restores the previous local file. That covers the local path, which is whatbake_layers,/editand the converter actually open.It does not cover this ordering:
os.replace(staged, final)— succeedsstorage.put_stream(...)— succeedsdb.commit()— failsLocal file and row are rolled back to the previous model; object storage keeps the newer bytes. The helper's docstring states this explicitly rather than leaving it implied.
Why it is bounded today
It is still real, and the fix is a design change rather than a guard.
What closing it properly requires
Publish each model as an immutable versioned artifact and move the pointer last, so nothing is ever overwritten in place and a failure at any step leaves the previous version wholly intact:
<project>/models/<version>/source.ifc) instead of the fixed<project>/source.ifc;Project.source_ifc(or a successor column) naming a version, with the local path derived from it;Blast radius (why it was not folded into #557)
Four things assume a fixed path or key today:
aec_api.storage— the<project>/source.ifckey is written and read in several places;edit_history— the existing per-edit versioning scheme, which a model-version scheme has to agree with rather than duplicate.PR #557 was four review rounds deep on a different defect. Landing a half-version of this inside it would have left two versioning schemes and a fixed key, which is worse than the single named window.
Done looks like
test_ifc_publish_atomicgains a check for the storage arm equivalent to the local one it has now (patchdb.committo raise after a successfulput_stream, assert storage still serves the old bytes);