Clean up

apps/LoopThroughWithOpenGLCompositing/runtime/persistence/PersistenceWriter.cpp

@@ -15,7 +15,8 @@ PersistenceWriter::PersistenceWriter(std::chrono::milliseconds debounceDelay, Sn
 
 PersistenceWriter::~PersistenceWriter()
 {
-	StopAndFlush();
+	std::string error;
+	StopAndFlush((std::chrono::milliseconds::max)(), error);
 }
 
 void PersistenceWriter::SetResultCallback(ResultCallback callback)
@@ -71,12 +72,6 @@ bool PersistenceWriter::EnqueueSnapshot(const PersistenceSnapshot& snapshot, std
 	return true;
 }
 
-void PersistenceWriter::StopAndFlush()
-{
-	std::string error;
-	StopAndFlush((std::chrono::milliseconds::max)(), error);
-}
-
 bool PersistenceWriter::StopAndFlush(std::chrono::milliseconds timeout, std::string& error)
 {
 	{

apps/LoopThroughWithOpenGLCompositing/runtime/persistence/PersistenceWriter.h

@@ -46,7 +46,6 @@ public:
 	bool WriteSnapshot(const PersistenceSnapshot& snapshot, std::string& error);
 	bool EnqueueSnapshot(const PersistenceSnapshot& snapshot, std::string& error);
 	bool StopAndFlush(std::chrono::milliseconds timeout, std::string& error);
-	void StopAndFlush();
 	PersistenceWriterMetrics GetMetrics() const;
 
 private:

apps/LoopThroughWithOpenGLCompositing/runtime/telemetry/HealthTelemetry.h

@@ -5,9 +5,8 @@
 #include <cstdint>
 #include <string>
 
-// Phase 1 compatibility seam for status and timing reporting. HealthTelemetry
+// HealthTelemetry owns the current operational status snapshot directly, so
-// owns the current operational status snapshot directly, so callers can report
+// callers can report health without sharing runtime-store state.
-// health without sharing runtime-store state.
 class HealthTelemetry
 {
 public:

docs/ARCHITECTURE_RESILIENCE_REVIEW.md

@@ -260,17 +260,22 @@ Recommended direction:
 
 ### 7. Persistence should be more asynchronous and debounced
 
-`SavePersistentState()` is still called directly from many update paths.
+Status: addressed by Phase 6.
 
 Relevant code:
 
-- `RuntimeHost.cpp`
+- `RuntimeCoordinator.cpp`
+- `RuntimeUpdateController.cpp`
+- `RuntimeStore.cpp`
+- `PersistenceWriter.cpp`
 
-Recent OSC work already reduced this problem for live automation, but the broader architecture would still benefit from:
+Runtime-state persistence now flows from accepted coordinator mutations to typed persistence events, then into a debounced background writer. The store still owns serialization and preset IO, while the writer owns temp-file replacement, coalescing, result reporting, and shutdown flushing.
 
-- a debounced persistence queue
+The remaining architecture concern is broader persistence policy, not direct mutation-path disk writes:
-- atomic write-behind snapshots
+
-- clear separation between state mutation and disk flush
+- whether preset saves should stay synchronous
+- whether runtime config writes should share the persistence writer
+- whether failed writes should retry automatically or wait for the next request
 
 This improves both resilience and timing safety.
 

docs/PHASE_6_BACKGROUND_PERSISTENCE_DESIGN.md

@@ -23,7 +23,7 @@ Current persistence footholds:
 
 Synchronous persistence is a poor fit for live software. A mutation that changes state should not also have to block on filesystem timing, antivirus scans, slow disks, or transient IO failures. The app needs persistence to be reliable and observable, but not timing-sensitive.
 
-The resilience review calls this out because `SavePersistentState()` style behavior can create unnecessary stalls and makes recovery harder to reason about.
+The resilience review calls this out because synchronous save-after-mutate behavior can create unnecessary stalls and makes recovery harder to reason about.
 
 Phase 6 should turn persistence into:
 
@@ -209,8 +209,8 @@ Current implementation:
 
 - `RuntimeStore::BuildRuntimeStatePersistenceSnapshot(...)` captures serialized runtime-state content and target path.
 - `PersistenceWriter::WriteSnapshot(...)` owns the temp-file and replace write mechanics.
-- `RuntimeStore::SavePersistentState(...)` still behaves synchronously, but now writes through `PersistenceWriter`.
+- Runtime-state persistence now flows through `RuntimeStore::RequestPersistence(...)` and the background writer.
-- Stack preset saves also use `PersistenceWriter` synchronously; preset async policy remains a later decision.
+- Stack preset saves still use `PersistenceWriter` synchronously; preset async policy remains a later decision.
 
 ### Step 3. Add Debounced Background Worker
 
@@ -226,7 +226,7 @@ Current implementation:
 
 - `PersistenceWriter::EnqueueSnapshot(...)` starts a worker lazily and debounces snapshots by `debounceKey`.
 - Runtime-state saves enqueue debounced snapshots, so routine mutation paths no longer write the runtime-state file directly.
-- Synchronous `PersistenceWriter::WriteSnapshot(...)` remains for stack preset saves and transitional direct writes.
+- Synchronous `PersistenceWriter::WriteSnapshot(...)` remains for stack preset saves.
 - `PersistenceWriterTests` use an injected in-memory sink to verify coalescing and non-coalesced immediate requests without touching the filesystem.
 
 ### Step 4. Add Atomic Write And Failure Reporting

docs/subsystems/RuntimeStore.md

@@ -187,7 +187,8 @@ Expected responsibilities:
 
 - `LoadConfig()`
 - `LoadPersistentState()`
-- `SavePersistentStateSnapshot(...)`
+- `BuildPersistentStateSnapshot(...)`
+- `RequestPersistence(...)`
 - `LoadStackPreset(...)`
 - `SaveStackPreset(...)`
 - `GetStackPresetNames()`
@@ -196,7 +197,7 @@ Design notes:
 
 - `Load*` operations should parse and normalize external file content into durable in-memory models.
 - `Save*` operations should serialize durable models without needing render or control subsystem context.
-- later debounce/background writing should wrap these operations, not redefine their ownership
+- debounce/background writing wraps these operations rather than redefining store ownership
 
 ### Read Interface
 
@@ -406,14 +407,14 @@ Target behavior:
 - serialization snapshots are built from those models
 - save requests persist a coherent snapshot
 
-This matters because the current code still calls `SavePersistentState()` directly from many mutation paths. That is one of the architectural pressure points already called out in [ARCHITECTURE_RESILIENCE_REVIEW.md](/c:/Users/Aiden/Documents/GitHub/video-shader-toys/docs/ARCHITECTURE_RESILIENCE_REVIEW.md).
+This matters because earlier code called persistent-state saves directly from mutation paths. Phase 6 removed that pressure point: accepted durable mutations now publish persistence requests, and `RuntimeStore::RequestPersistence(...)` builds a coherent snapshot for the background writer.
 
 The Phase 1 design for `RuntimeStore` should therefore assume:
 
 - store ownership of serialization remains
-- immediate save-after-mutate is a migration detail, not the final behavioral contract
+- persistence requests, not mutation methods, are the durable write boundary
 
-By Phase 6, a background snapshot writer may sit underneath or beside this subsystem, but the durable model still belongs here.
+Phase 6 added that background snapshot writer underneath this subsystem, while keeping the durable model here.
 
 ## Migration Plan From Current Code
 

tests/PersistenceWriterTests.cpp

@@ -54,7 +54,7 @@ void TestDebouncedRequestsCoalesceToNewestSnapshot()
 	Expect(metrics.enqueuedCount == 1, "first debounced snapshot counts as enqueue");
 	Expect(metrics.coalescedCount == 1, "second debounced snapshot counts as coalesced");
 
-	writer.StopAndFlush();
+	Expect(writer.StopAndFlush(std::chrono::seconds(1), error), "flush drains debounced snapshots");
 
 	{
 		std::lock_guard<std::mutex> lock(mutex);
@@ -87,7 +87,7 @@ void TestImmediateRequestsAreNotCoalesced()
 	std::string error;
 	Expect(writer.EnqueueSnapshot(first, error), "first immediate snapshot enqueues");
 	Expect(writer.EnqueueSnapshot(second, error), "second immediate snapshot enqueues");
-	writer.StopAndFlush();
+	Expect(writer.StopAndFlush(std::chrono::seconds(1), error), "flush drains immediate snapshots");
 
 	{
 		std::lock_guard<std::mutex> lock(mutex);
@@ -116,7 +116,7 @@ void TestWriteFailureReportsStructuredResult()
 
 	std::string error;
 	Expect(writer.EnqueueSnapshot(snapshot, error), "failing snapshot still enqueues");
-	writer.StopAndFlush();
+	Expect(writer.StopAndFlush(std::chrono::seconds(1), error), "flush reports failing snapshot result");
 
 	Expect(results.size() == 1, "writer reports one failure result");
 	Expect(!results.empty() && !results[0].succeeded, "writer result records failure");