#268 — Atomic (transactional) plot save: commitPlotSave / reconcilePlotSave

BACKLOG.md

@@ -180,7 +180,7 @@ Description formats:
 | 269 | Tech Debt | Split `SceneProperties` into `InstantSceneProperties` + `TimeRangeSceneProperties` (LinkML refactor) — introduce two distinct LinkML classes (`InstantScene` and `TimeRangeScene`) with a `kind` discriminator, replacing the current XOR-rule + TS-predicate discrimination layered on top of a single `SceneProperties` class. The XOR rule lives in three places today after #263 (LinkML rule, `flavourCheck` in `validate.ts`, `isTimeRangeScene` predicate in `types.ts`); two classes with a discriminator make the flavour-coupling structural, eliminate the cross-field rule, give LinkML→Pydantic→TS clean discriminated unions for free, and remove the "narrow at the predicate" rule from contributor cognition. Considered and deferred during #263 research (research.md R2) because the renaming cost was disproportionate to a two-line XOR rule. Once a third Scene flavour arrives (e.g. live-link / streamed Scenes), the discriminator approach becomes strictly cheaper. Probably belongs in the v3 schema cycle when a breaking change is allowed. Estimate 5–8 dev-days. (follow-up to #263 review decision; depends on #263 shipping + likely sequenced post-v4.0.0 per Article XIV) | 2 | 1 | 3 | 6 | Medium | blocked |  | 2026-05-19 | 2026-05-26 |
 | ~~263~~ | ~~Enhancement~~ | ~~[Storyboard time-range Scenes — `time_range != null` interpolation (pan/zoom + time-slider scrub)](specs/263-time-range-scenes/spec.md) — lift the v1 single-instant constraint and make `time_range != null` a first-class capture + playback mode. Capture extends #216 with a "range" affordance recording `[t_start, t_end]` + a `viewport_end` snapshot. Playback extends #217: during `executeTransition` into a time-range Scene the engine simultaneously interpolates the viewport `viewport_start → viewport_end` AND advances the time-slider `t_start → t_end` over the captured wall-clock duration, so feature visibility / track positions / chart cursors move in lock-step. Reverse playback reverses both axes. Schema work bumps `geojson.yaml` (`SceneProperties.time_range` already exists; add `viewport_end`); generate Pydantic/TS/JSON Schema; Article II adherence tests + new golden fixtures for both Scene flavours. Linear interpolation only in MVP; ease-in/ease-out and edit-time range adjustment deferred. **Lands before #264** per interview sequencing. (split from #229 via `/interview 229`; requires #215–#218 MVP; [issue #630](https://github.com/debrief/debrief-future/issues/630))~~ | ~~4~~ | ~~4~~ | ~~3~~ | ~~11~~ | ~~Medium~~ | ~~complete~~ | ~~E13~~ | ~~2026-05-19~~ | ~~2026-05-19~~ |
 | ~~264~~ | ~~Feature~~ | ~~[Air-gapped briefing zip — self-contained distraction-free Storyboard renderer (SPA)](specs/264-briefing-zip-renderer/spec.md) — ship the briefing renderer as a self-contained zip that opens in any modern browser by double-clicking `index.html`: no server, no extension host, no network calls. Contents = bundled SPA + `features.geojson` (which already contains the `StoryboardFeature` / `SceneFeature` entries — Storyboards live inside the plot's GeoJSON per `storyboard.yaml`, not as a separate document) + `item.json` + Scene-thumbnail assets + pre-fetched basemap tiles. Build command takes a Storyboard ID (one plot may contain multiple Storyboards). Single Storyboard per zip, read-only, with Present (chrome-hidden) / Minimal (transport + scrub) toggle. Reuses the shared playback engine from #217 + #258 displayMode capture. New VS Code command "Export Storyboard as briefing zip…" on the Storyboard overflow menu. New SPA at `apps/briefing-renderer/`. **Blocked on #263** (time-range Scenes) per interview sequencing — briefing renderer should ship knowing time-range is in play to avoid rework on the shared engine. (split from #229 via `/interview 229`; requires #215–#218 MVP, #174 thumbnails, #258 displayMode capture; [issue #631](https://github.com/debrief/debrief-future/issues/631))~~ | ~~4~~ | ~~5~~ | ~~3~~ | ~~12~~ | ~~Medium~~ | ~~complete~~ | ~~E13~~ | ~~2026-05-19~~ | ~~2026-05-20~~ |
-| 268 | Tech Debt | VS Code save atomicity (broader) — `apps/vscode/src/commands/saveSession.ts:163–208` currently performs three separate write operations (sidecar at 163, FeatureCollection at 178, thumbnails at 184–203) with non-blocking error handling at lines 180 and 192–202. After #249/spec-261 lands FR-019, the FC↔sidecar pair on the *migrated-state* path is sequenced FC-first/sidecar-second to close the silent-failure gap for that specific path — but the broader flow (thumbnails, STAC assets, sidecar, FC) is still independently committable. A real production failure mid-sequence could leave any subset of files updated. This ticket scopes a broader transactional-save model: either (a) a save-orchestrator that stages all writes to a tmp directory and atomically moves them into place, or (b) an explicit "last good known state" recovery file so a half-completed save can be rolled back on the next open. Estimate 5–8 dev-days; medium-to-high complexity (touches every save consumer). Trigger: a user reports a partial-save corruption, OR a security/audit review flags the non-atomic save flow. (follow-up to #249; cross-references docs/project_notes/decisions.md re: writer abstraction; potentially extends Constitution Article IV.4) | - | - | - | - | High | approved |  | 2026-05-19 | 2026-05-31 |
+| ~~268~~ | ~~Tech Debt~~ | ~~[VS Code save atomicity (broader)](specs/268-save-atomicity/spec.md) — after the #249/spec-261 sidecar retirement, `apps/vscode/src/commands/saveSession.ts` performs **two** independent, non-transactional write phases (was three — the `.debrief-session` sidecar write is gone): (1) `storeFeatureCollection()` (line 124) writes `features.geojson`, with view-state (viewport, time window/playhead, selection) folded in as `SystemState` features + per-feature `visible` flags by `applyStateToFeatures` first (line 123); (2) `storeThumbnails()` (line 142) drives `StacWriter.writePlotThumbnailPair`, which writes the large + small thumbnail PNGs and updates the STAC item JSON. The two phases are independently committable and ordered badly for failure: the FC write hard-fails correctly (error surfaced, early `return`, dirty flag retained — catch at lines 125–130), but the dirty flag is then cleared and "Plot saved" is shown (lines 133–134) *before* the thumbnail/STAC write runs — so a `StacWriterError` there (surfaced at 147–148; non-`StacWriterError` capture failures are non-blocking at line 150) leaves a plot that looks saved (clean dirty flag, success toast) but carries stale/missing thumbnails, and a crash between phases leaves `features.geojson` updated against an old STAC item. A real production failure between the phases leaves a partial on-disk state. This ticket scopes a transactional-save model: either (a) a save-orchestrator that stages all writes to a tmp location and atomically moves them into place, or (b) an explicit "last good known state" recovery file so a half-completed save can be rolled back on the next open. Note the host-agnostic `StacWriter` spans the FS adaptor *and* the web-shell IndexedDB store, so "atomic move" needs a writer-level transaction/commit abstraction, not a bare `fs.rename` — which is what keeps this medium-to-high despite dropping to two writes. Estimate ~5–7 dev-days (marginally less than the original 5–8 now the sidecar write is gone; the bulk is the cross-host transaction model, not the write count); medium-to-high complexity. Trigger: a user reports a partial-save corruption, OR a security/audit review flags the non-atomic save flow. (rescoped 2026-06-01 after the spec-261 sidecar retirement landed; follow-up to #249/spec-261; cross-references docs/project_notes/decisions.md re: writer abstraction; potentially extends Constitution Article IV.4)~~ | ~~-~~ | ~~-~~ | ~~-~~ | ~~-~~ | ~~High~~ | ~~complete~~ |  | ~~2026-05-19~~ | ~~2026-06-01~~ |
 | ~~267~~ | ~~Tech Debt~~ | ~~[Out-of-window `current_time` policy — #249/spec-261 introduces an optional `current_time` field on the LinkML `SystemStateProperties` `temporal` variant (the saved playhead position) and ships strict-on-import validation: if a plot's saved `current_time` falls outside its `[start_time, end_time]` window, the load fails with `SystemStateLoadError(kind='cross-field-invariant')`. This is the right Article XIV.4 default for v4.x pre-release. But if analyst feedback later shows that malformed plots arrive often enough to be a friction (e.g. a colleague trims the analytical window after scrubbing, leaving the playhead orphaned), the team may want a tolerant alternative: clamp to the nearest window edge with a non-modal toast, OR offer "open ignoring saved playhead" as a fallback action. This ticket revisits the policy if/when the strict-on-import behaviour proves user-hostile. Likely depends on real-world usage data from #249 ship. Estimate 1–2 dev-days. Trigger: analyst feedback that plots are failing to load due to out-of-window playheads, OR a deliberate UX decision post-v4.0.0 to add tolerant import paths (Article XIV trigger point). (follow-up to #249; depends on real-world data)](specs/267-tolerant-playhead-import/spec.md)~~ | ~~2~~ | ~~1~~ | ~~4~~ | ~~7~~ | ~~Low~~ | ~~complete~~ |  | ~~2026-05-19~~ | ~~2026-05-29~~ |
 | ~~266~~ | ~~Tech Debt~~ | ~~Spatial-shape cleanup follow-up to #249 — #249/spec-261 unifies the LinkML schema's spatial-viewport representation onto `ViewportPolygon`, removing the parallel `bbox: float[4]` + `zoom: float` + `center: float[2]` fields from `SystemStateProperties` (per Article XIV.1 — verified zero runtime blast radius today). If any docs, ADRs, ARCHITECTURE.md sections, code comments, or external references still mention the `bbox`/`center` shape after #249 ships, they need updating. Likely affected: `docs/project_notes/decisions.md` (any ADR referencing the spatial SystemState shape), `ARCHITECTURE.md` schema-extension passages, MCP envelope docs if any, and any spec for #215 / #237 that referenced the bbox shape. Estimate 0.5–1 dev-day. Trigger: #249 has shipped and a grep for `SystemStateProperties.*bbox` / `state_type.*spatial.*bbox` over the repo turns up stale references. (follow-up to #249; cross-references shared/schemas/src/linkml/geojson.yaml post-#249)~~ | ~~-~~ | ~~-~~ | ~~-~~ | ~~-~~ | ~~Low~~ | ~~complete~~ |  | ~~2026-05-19~~ | ~~2026-06-01~~ |
 | 265 | Research Spike | [MP4 / GIF export of a Storyboard traversal — encoding-pipeline trade-off spike](https://github.com/debrief/debrief-future/issues/632) — time-boxed (1–2 days) research evaluating at least three encoding-pipeline candidates: (1) client-side `MediaRecorder` + `ffmpeg.wasm` (~20MB bundle bloat, fully offline, ships inside the briefing zip too), (2) service-side Python `services/export/` + system `ffmpeg` (clean Article IV.1, faster, but new system dep + doesn't ship inside the air-gapped briefing zip), (3) hybrid using WebCodecs API with `ffmpeg.wasm` GIF fallback. Deliverable = `docs/project_notes/storyboard-video-export-research.md` with bundle-size measurements, encoding-time benchmarks against a 10-Scene 1080p Storyboard, output-quality samples under `evidence/`, Article I compatibility analysis, briefing-zip (#264) integration story per option, recommendation + a follow-up implementation backlog item. **No shipping code from this ticket.** (split from #229 via `/interview 229`; independent of #263/#264 but recommendation should consider both) | 2 | 2 | 4 | 8 | Low | approved | E13 | 2026-05-19 | 2026-05-19 |

CLAUDE.md

@@ -218,6 +218,7 @@ Only updated when a feature introduces a technology not already listed here.
 - Python 3.11 (services, schema-build tooling, + LinkML ≥ 1.7.0 (schema source + (223-linkml-stac-catalog)
 - TypeScript 5.x (strict mode) — no new technology; amends spec-261's `@debrief/session-state` `SystemState` load layer (`validate.ts`, reconciliation, `load.ts`) + host notification surfaces. No schema change, no new dependency. (267-tolerant-playhead-import)
 - TypeScript 5.x (strict), Node 20 runtime (VS Code extension host); React 18.x (renderer + panel) + `@debrief/components` (StoryboardPanel), `@debrief/stac-writer` (web-shell reads), `@debrief/schemas` (LinkML types — consumed, not changed), JSZip 3.10.1 (already present), VS Code Extension API ^1.85.0, `react-leaflet`/Leaflet (renderer map), Vite 5.x (web-shell + renderer) (273-storyboard-preview-button)
+- TypeScript 5.x (strict; Article XV). No Python change. + `@debrief/stac-writer` (interface + core), `node:fs`/`node:crypto` (fs adaptor — existing), `idb` (web-shell adaptor — existing), `@debrief/schemas` (`FeatureCollection`, `StacItem`), VS Code Extension API (`window.showWarningMessage`). **No new runtime dependencies.** (268-save-atomicity)
 
 ## Before Pushing
 
@@ -301,5 +302,6 @@ Each wrapper:
 Note: `vitest` does not catch TypeScript type errors — only `tsc` (run during typecheck) does. The `pnpm build` step also runs `tsc`, but typecheck is the explicit CI gate.
 
 ## Recent Changes
+- 268-save-atomicity: Added TypeScript 5.x (strict; Article XV). No Python change. + `@debrief/stac-writer` (interface + core), `node:fs`/`node:crypto` (fs adaptor — existing), `idb` (web-shell adaptor — existing), `@debrief/schemas` (`FeatureCollection`, `StacItem`), VS Code Extension API (`window.showWarningMessage`). **No new runtime dependencies.**
 - 267-tolerant-playhead-import: Relaxes spec-261 FR-018 strict-on-import for one recoverable case — orphaned playhead (out-of-window `current_time`) clamps to nearest window edge + non-blocking notification; incoherent window (`start>end`) still hard-fails. No schema change, no new dependency.
 - 273-storyboard-preview-button: Added TypeScript 5.x (strict), Node 20 runtime (VS Code extension host); React 18.x (renderer + panel) + `@debrief/components` (StoryboardPanel), `@debrief/stac-writer` (web-shell reads), `@debrief/schemas` (LinkML types — consumed, not changed), JSZip 3.10.1 (already present), VS Code Extension API ^1.85.0, `react-leaflet`/Leaflet (renderer map), Vite 5.x (web-shell + renderer)

apps/vscode/src/commands/openPlot.ts

@@ -8,6 +8,7 @@ import { createLogService, createSnapshotService, createTimeInstant, type Result
 import { hydrateStoreFromFeatures, SystemStateLoadError, type PlayheadClampDiagnostic } from '../services/systemStateBridge';
 import { notifyPlayheadClamps } from '../services/playheadClampNotice';
 import type { StacWriter } from '@debrief/stac-writer';
+import { reconcileBeforeOpen } from './reconcileOnOpen';
 import type { ConfigService } from '../services/configService';
 import type { StacService } from '../services/stacService';
 import type { CalcService } from '../services/calcService';
@@ -134,6 +135,12 @@ export function createOpenPlotCommand(
       return;
     }
 
+    // #268 — reconcile any interrupted save BEFORE reading the plot, so the
+    // loads below observe a single coherent on-disk state (FR-007/FR-008).
+    await reconcileBeforeOpen(getStacWriter, store.path, itemPath, (message) => {
+      void vscode.window.showWarningMessage(message);
+    });
+
     // Load plot
     const plot = await vscode.window.withProgress(
       {

apps/vscode/src/commands/reconcileOnOpen.ts

@@ -0,0 +1,42 @@
+/**
+ * Reconcile-on-open hook for the atomic plot save (#268, US3).
+ *
+ * Extracted from `openPlot.ts` so it can be exercised in a Node test
+ * environment without pulling in the webview/Leaflet dependency graph.
+ */
+
+import type { StacWriter, ReconcilePlotSaveResult } from '@debrief/stac-writer';
+
+/**
+ * Heal an interrupted save before the plot is read. `reconcilePlotSave` can
+ * change what is on disk (roll a committed-but-unapplied save forward, or
+ * discard pre-commit temps), so this MUST run before `loadPlot` /
+ * `loadPlotData`. When it acts, a non-blocking notice is shown (FR-008).
+ * Reconcile must never block opening — any failure is logged and swallowed.
+ */
+export async function reconcileBeforeOpen(
+  getStacWriter: ((storePath: string) => StacWriter) | undefined,
+  storePath: string,
+  itemPath: string,
+  showWarning: (message: string) => void,
+): Promise<ReconcilePlotSaveResult | null> {
+  if (!getStacWriter) {
+    return null;
+  }
+  try {
+    const writer = getStacWriter(storePath);
+    const result = await writer.reconcilePlotSave({
+      ctx: { kind: 'fs', nowMs: () => Date.now(), randomId: () => '' },
+      stacItemPath: itemPath,
+    });
+    if (result.recovered) {
+      showWarning(
+        'Recovered an interrupted save — opened the last good version of this plot.',
+      );
+    }
+    return result;
+  } catch (err) {
+    console.warn('[debrief] openPlot: reconcilePlotSave failed', err);
+    return null;
+  }
+}