diff --git a/packages/playground/lib/features/ai/quick_agent/core/engine/services/deck_generator_service.dart b/packages/playground/lib/features/ai/quick_agent/core/engine/services/deck_generator_service.dart index e6af5410..f4ef293e 100644 --- a/packages/playground/lib/features/ai/quick_agent/core/engine/services/deck_generator_service.dart +++ b/packages/playground/lib/features/ai/quick_agent/core/engine/services/deck_generator_service.dart @@ -345,6 +345,33 @@ class DeckGeneratorService { ); } + /// Runs one generation entry point against a fresh model client. + /// + /// Creates the client, builds its call executor, loads the prompts, and + /// closes the client when [body] completes or throws. Every entry point uses + /// this helper so they share one setup and one disposal path. + Future _withModelSession( + DeckGenerationRequest request, { + required GenerationTraceEmitter trace, + required bool Function() isCancelled, + required Future Function(GenerationModelCallExecutor executor) body, + }) async { + final client = _modelClientFactory(apiKey); + try { + final executor = _createExecutor( + client: client, + trace: trace, + request: request, + isCancelled: isCancelled, + ); + await _promptProvider.load(); + + return await body(executor); + } finally { + client.close(); + } + } + /// Generates and validates the shared deck plan without composing slides. Future plan( DeckGenerationRequest request, { @@ -369,36 +396,33 @@ class DeckGeneratorService { final pipelineStart = DateTime.now(); final trace = GenerationTraceEmitter(onTrace); bool generationCancelled() => isCancelled?.call() ?? false; - GenerationModelClient? client; - try { - client = _modelClientFactory(apiKey); - final executor = _createExecutor( - client: client, + return await _withModelSession( + request, trace: trace, - request: request, isCancelled: generationCancelled, + body: (executor) async { + final outline = await _runOutlinePhase( + this, + executor: executor, + prompt: modelInput, + request: request, + themeCandidates: themeCandidates, + onProgress: onProgress, + trace: trace, + ); + if (generationCancelled()) { + return const DeckPlanningResult.failure('Generation cancelled.'); + } + if (outline == null) { + return const DeckPlanningResult.failure( + 'Failed to generate presentation outline. Please try again.', + ); + } + + return DeckPlanningResult.success(outline); + }, ); - await _promptProvider.load(); - final outline = await _runOutlinePhase( - this, - executor: executor, - prompt: modelInput, - request: request, - themeCandidates: themeCandidates, - onProgress: onProgress, - trace: trace, - ); - if (generationCancelled()) { - return const DeckPlanningResult.failure('Generation cancelled.'); - } - if (outline == null) { - return const DeckPlanningResult.failure( - 'Failed to generate presentation outline. Please try again.', - ); - } - - return DeckPlanningResult.success(outline); } on GenerationCancelledException { return const DeckPlanningResult.failure('Generation cancelled.'); } on GenerationBudgetExceededException catch (error, stack) { @@ -421,8 +445,6 @@ class DeckGeneratorService { return DeckPlanningResult.failure( const ErrorClassifier().getUserMessage(error), ); - } finally { - client?.close(); } } @@ -455,56 +477,53 @@ class DeckGeneratorService { final pipelineStart = DateTime.now(); final trace = GenerationTraceEmitter(onTrace); bool generationCancelled() => isCancelled?.call() ?? false; - GenerationModelClient? client; - try { - client = _modelClientFactory(apiKey); - final executor = _createExecutor( - client: client, - trace: trace, - request: request, - isCancelled: generationCancelled, - ); - await _promptProvider.load(); - final images = await _runImagePhase( - this, - plan: approvedPlan, - request: request, - onProgress: onProgress, + return await _withModelSession( + request, trace: trace, isCancelled: generationCancelled, - ); - if (generationCancelled()) { - return DeckGenerationResult.failure('Generation cancelled.'); - } - final composition = await _runSlideCompositionPhase( - this, - executor: executor, - prompt: modelInput, - request: request, - outline: images.plan, - onProgress: onProgress, - trace: trace, - isCancelled: isCancelled, - ); - if (generationCancelled()) { - return DeckGenerationResult.failure('Generation cancelled.'); - } - if (composition == null) { - return DeckGenerationResult.failure( - 'Failed while composing presentation slides. Please try again.', - ); - } - - return _finalizeDeck( - this, - composition: composition, - plan: images.plan, - generatedImages: images.assets, - pipelineStart: pipelineStart, - onProgress: onProgress, - isCancelled: isCancelled, - trace: trace, + body: (executor) async { + final images = await _runImagePhase( + this, + plan: approvedPlan, + request: request, + onProgress: onProgress, + trace: trace, + isCancelled: generationCancelled, + ); + if (generationCancelled()) { + return DeckGenerationResult.failure('Generation cancelled.'); + } + final composition = await _runSlideCompositionPhase( + this, + executor: executor, + prompt: modelInput, + request: request, + outline: images.plan, + onProgress: onProgress, + trace: trace, + isCancelled: isCancelled, + ); + if (generationCancelled()) { + return DeckGenerationResult.failure('Generation cancelled.'); + } + if (composition == null) { + return DeckGenerationResult.failure( + 'Failed while composing presentation slides. Please try again.', + ); + } + + return _finalizeDeck( + this, + composition: composition, + plan: images.plan, + generatedImages: images.assets, + pipelineStart: pipelineStart, + onProgress: onProgress, + isCancelled: isCancelled, + trace: trace, + ); + }, ); } on GenerationCancelledException { return DeckGenerationResult.failure('Generation cancelled.'); @@ -527,8 +546,6 @@ class DeckGeneratorService { return DeckGenerationResult.failure( const ErrorClassifier().getUserMessage(error), ); - } finally { - client?.close(); } } @@ -580,68 +597,65 @@ class DeckGeneratorService { final pipelineStart = DateTime.now(); final trace = GenerationTraceEmitter(onTrace); bool generationCancelled() => isCancelled?.call() ?? false; - GenerationModelClient? client; - try { - client = _modelClientFactory(apiKey); - final executor = _createExecutor( - client: client, - trace: trace, - request: request, - isCancelled: generationCancelled, - ); - await _promptProvider.load(); - onProgress?.call(const GenerationProgress(.composingSlides)); - final existingSlidesByKey = { - for (final slide in partialResult.slides) - slide.key: Map.of(slide.toJson()), - }; - final retried = await _composeSlidesSequentially( - executor, - modelInput, - plan, + return await _withModelSession( request, - trace, - onProgress, - generationCancelled, - targetSlideKeys: retryableKeys, - existingSlidesByKey: existingSlidesByKey, - ); - if (generationCancelled()) { - return DeckGenerationResult.failure('Generation cancelled.'); - } - if (retried == null) { - return DeckGenerationResult.failure( - 'Failed while retrying unresolved slides. Please try again.', - ); - } - - final mergedByKey = >{ - ...existingSlidesByKey, - for (final slide in retried.slides) slide['key']! as String: slide, - }; - final remainingFailures = [ - for (final failure in partialResult.slideFailures) - if (!failure.retryable) failure, - ...retried.failures, - ]; - final merged = _SlideCompositionResult( - slides: List.unmodifiable([ - for (final plannedSlide in plan.slides) - ?mergedByKey[plannedSlide.key], - ]), - failures: List.unmodifiable(remainingFailures), - ); - - return _finalizeDeck( - this, - composition: merged, - plan: plan, - generatedImages: partialResult.generatedImages, - pipelineStart: pipelineStart, - onProgress: onProgress, - isCancelled: generationCancelled, trace: trace, + isCancelled: generationCancelled, + body: (executor) async { + onProgress?.call(const GenerationProgress(.composingSlides)); + final existingSlidesByKey = { + for (final slide in partialResult.slides) + slide.key: Map.of(slide.toJson()), + }; + final retried = await _composeSlidesSequentially( + executor, + modelInput, + plan, + request, + trace, + onProgress, + generationCancelled, + targetSlideKeys: retryableKeys, + existingSlidesByKey: existingSlidesByKey, + ); + if (generationCancelled()) { + return DeckGenerationResult.failure('Generation cancelled.'); + } + if (retried == null) { + return DeckGenerationResult.failure( + 'Failed while retrying unresolved slides. Please try again.', + ); + } + + final mergedByKey = >{ + ...existingSlidesByKey, + for (final slide in retried.slides) slide['key']! as String: slide, + }; + final remainingFailures = [ + for (final failure in partialResult.slideFailures) + if (!failure.retryable) failure, + ...retried.failures, + ]; + final merged = _SlideCompositionResult( + slides: List.unmodifiable([ + for (final plannedSlide in plan.slides) + ?mergedByKey[plannedSlide.key], + ]), + failures: List.unmodifiable(remainingFailures), + ); + + return _finalizeDeck( + this, + composition: merged, + plan: plan, + generatedImages: partialResult.generatedImages, + pipelineStart: pipelineStart, + onProgress: onProgress, + isCancelled: generationCancelled, + trace: trace, + ); + }, ); } on GenerationCancelledException { return DeckGenerationResult.failure('Generation cancelled.'); @@ -664,8 +678,6 @@ class DeckGeneratorService { return DeckGenerationResult.failure( const ErrorClassifier().getUserMessage(error), ); - } finally { - client?.close(); } } @@ -706,76 +718,72 @@ class DeckGeneratorService { bool generationCancelled() => isCancelled?.call() ?? false; DeckGenerationResult cancelledResult() => DeckGenerationResult.failure('Generation cancelled.'); - GenerationModelClient? client; - try { - client = _modelClientFactory(apiKey); - final executor = _createExecutor( - client: client, - trace: trace, - request: request, - isCancelled: generationCancelled, - ); - await _promptProvider.load(); - - final outline = await _runOutlinePhase( - this, - executor: executor, - prompt: modelInput, - request: request, - themeCandidates: themeCandidates, - onProgress: onProgress, - trace: trace, - ); - if (generationCancelled()) { - return cancelledResult(); - } - if (outline == null) { - return DeckGenerationResult.failure( - 'Failed to generate presentation outline. Please try again.', - ); - } - - final images = await _runImagePhase( - this, - plan: outline, - request: request, - onProgress: onProgress, + return await _withModelSession( + request, trace: trace, isCancelled: generationCancelled, - ); - if (generationCancelled()) { - return cancelledResult(); - } - final composition = await _runSlideCompositionPhase( - this, - executor: executor, - prompt: modelInput, - request: request, - outline: images.plan, - onProgress: onProgress, - trace: trace, - isCancelled: isCancelled, - ); - if (generationCancelled()) { - return cancelledResult(); - } - - if (composition == null) { - return DeckGenerationResult.failure( - 'Failed while composing presentation slides. Please try again.', - ); - } - - return _finalizeDeck( - this, - composition: composition, - plan: images.plan, - generatedImages: images.assets, - pipelineStart: pipelineStart, - onProgress: onProgress, - isCancelled: isCancelled, - trace: trace, + body: (executor) async { + final outline = await _runOutlinePhase( + this, + executor: executor, + prompt: modelInput, + request: request, + themeCandidates: themeCandidates, + onProgress: onProgress, + trace: trace, + ); + if (generationCancelled()) { + return cancelledResult(); + } + if (outline == null) { + return DeckGenerationResult.failure( + 'Failed to generate presentation outline. Please try again.', + ); + } + + final images = await _runImagePhase( + this, + plan: outline, + request: request, + onProgress: onProgress, + trace: trace, + isCancelled: generationCancelled, + ); + if (generationCancelled()) { + return cancelledResult(); + } + final composition = await _runSlideCompositionPhase( + this, + executor: executor, + prompt: modelInput, + request: request, + outline: images.plan, + onProgress: onProgress, + trace: trace, + isCancelled: isCancelled, + ); + if (generationCancelled()) { + return cancelledResult(); + } + + if (composition == null) { + return DeckGenerationResult.failure( + 'Failed while composing presentation slides. Please try again.', + ); + } + + return _finalizeDeck( + this, + composition: composition, + plan: images.plan, + generatedImages: images.assets, + pipelineStart: pipelineStart, + onProgress: onProgress, + isCancelled: isCancelled, + trace: trace, + ); + }, ); } on GenerationCancelledException { return cancelledResult(); @@ -796,8 +804,6 @@ class DeckGeneratorService { ); final userMessage = const ErrorClassifier().getUserMessage(e); return DeckGenerationResult.failure(userMessage); - } finally { - client?.close(); } } } diff --git a/packages/playground/lib/features/ai/quick_agent/domain/commands/generate_deck_command.dart b/packages/playground/lib/features/ai/quick_agent/domain/commands/generate_deck_command.dart index 0f5b400f..5f05c05c 100644 --- a/packages/playground/lib/features/ai/quick_agent/domain/commands/generate_deck_command.dart +++ b/packages/playground/lib/features/ai/quick_agent/domain/commands/generate_deck_command.dart @@ -29,9 +29,16 @@ class GenerationException implements Exception { /// intermediate progress, which the base command's binary running state doesn't /// model. On success it serializes the slides to Markdown and replaces the /// shared [DeckDocumentStore]. +/// +/// A run is bound to the document revision and the file binding it started +/// from. Editing the document or switching decks discards the generated deck +/// and leaves [completionNotice] for the editor to show, because the panel +/// that started the run may already be closed. class GenerateDeckCommand extends Command1 { final GeneratedDeckResultApplier _resultApplier; + final DeckDocumentStore _documentStore; + final int Function()? _bindingRevision; final DeckGeneratorService? _service; GenerationProgress _progress = const GenerationProgress(GenerationPhase.idle); @@ -44,12 +51,15 @@ class GenerateDeckCommand extends Command1 { MemoryDeckLoader? deckLoader, MemoryAssetCacheStore? assetCacheStore, DeckGeneratorService? service, + int Function()? bindingRevision, }) : _resultApplier = GeneratedDeckResultApplier( documentStore: documentStore, deckLoader: deckLoader, assetCacheStore: assetCacheStore, customizationStore: customizationStore, ), + _documentStore = documentStore, + _bindingRevision = bindingRevision, _service = service; void _onProgress(GenerationProgress progress) { @@ -63,9 +73,19 @@ class GenerateDeckCommand extends Command1 { GenerationProgress get progress => _progress; - /// Non-blocking detail for a completed partial generation. + /// Non-blocking detail for a completed, partial, or discarded generation. + /// + /// The editor keeps showing it after the generation panel closes. Call + /// [dismissNotice] once the reader has seen it. String? get completionNotice => _completionNotice; + /// Clears the notice the editor shows. + void dismissNotice() { + if (_completionNotice == null) return; + _completionNotice = null; + notifyListeners(); + } + @override Future> action(DeckGenerationRequest request) async { if (_service == null && !EnvConfig.hasGeminiApiKey) { @@ -82,6 +102,14 @@ class GenerateDeckCommand extends Command1 { _progress = const GenerationProgress(GenerationPhase.generatingOutline); notifyListeners(); + final startDocumentRevision = _documentStore.revision; + final startBindingRevision = _bindingRevision?.call(); + bool isCurrentDeck() => + !_cancelled && + !_disposed && + _documentStore.revision == startDocumentRevision && + _bindingRevision?.call() == startBindingRevision; + try { final service = _service ?? DeckGeneratorService(apiKey: EnvConfig.geminiApiKey); @@ -105,7 +133,23 @@ class GenerateDeckCommand extends Command1 { return const Result.error(GenerationException('Generation cancelled.')); } - await _resultApplier.apply(result); + final application = await _resultApplier.apply( + result, + isValid: isCurrentDeck, + ); + if (!application.published) { + _completionNotice = + 'The generated deck was discarded because the document changed ' + 'while it was being created.'; + debugLog.log( + 'GENERATE_DECK', + 'Discarded ${result.slides.length} generated slides; ' + 'the editor moved on to newer work.', + ); + + return const Result.ok(null); + } + if (result.isPartial) { _completionNotice = result.error; } else if (result.hasImageFailures) { @@ -114,6 +158,19 @@ class GenerateDeckCommand extends Command1 { '${result.generatedImages.length} planned artworks; ' '${result.failedImageCount} used a text-first fallback.'; } + if (application.cleanupError case final cleanupError?) { + // The deck is published. Removing the artwork it replaced is a + // separate, non-blocking problem. + debugLog.error( + 'GENERATE_DECK', + 'Could not remove replaced artwork: $cleanupError', + .current, + ); + _completionNotice = [ + ?_completionNotice, + 'Some artwork from the previous deck could not be removed.', + ].join(' '); + } debugLog.log( 'GENERATE_DECK', 'Loaded ${result.slides.length} slides into editor' @@ -136,12 +193,6 @@ class GenerateDeckCommand extends Command1 { } } - @override - void clearResult() { - _completionNotice = null; - super.clearResult(); - } - @override void notifyListeners() { if (!_disposed) super.notifyListeners(); diff --git a/packages/playground/lib/features/ai/quick_agent/domain/generated_deck_result_applier.dart b/packages/playground/lib/features/ai/quick_agent/domain/generated_deck_result_applier.dart index cf049bcc..266bfded 100644 --- a/packages/playground/lib/features/ai/quick_agent/domain/generated_deck_result_applier.dart +++ b/packages/playground/lib/features/ai/quick_agent/domain/generated_deck_result_applier.dart @@ -1,3 +1,5 @@ +import 'dart:async'; + import 'package:superdeck_builder/superdeck_builder.dart'; import 'package:superdeck_core/superdeck_core.dart'; @@ -7,6 +9,27 @@ import '../../../editor/domain/stores/deck_document_store.dart'; import '../core/engine/services/deck_generator_service.dart'; import 'generated_deck_style_mapper.dart'; +/// Reports whether the generation that produced a result is still the one the +/// host wants published. +/// +/// The applier calls it before it stages artwork, and again after that +/// asynchronous work, immediately before it publishes the deck. +typedef GeneratedDeckApplicationGuard = bool Function(); + +/// Outcome of one [GeneratedDeckResultApplier.apply] call. +final class GeneratedDeckApplication { + /// Whether the document, preview, and theme received this result. + final bool published; + + /// Failure raised while removing artwork that the published deck replaced. + /// + /// The deck is published when this is set. Hosts report it separately from a + /// generation failure. + final Object? cleanupError; + + const GeneratedDeckApplication({required this.published, this.cleanupError}); +} + /// Applies generated decks for one host and evicts artwork from the deck it /// replaces only after the replacement has been published successfully. final class GeneratedDeckResultApplier { @@ -16,6 +39,10 @@ final class GeneratedDeckResultApplier { final AssetCacheStore? _assetCacheStore; final DeckCustomizationStore _customizationStore; Set _appliedAssetKeys = const {}; + + /// Serializes application, so two results for one host cannot interleave + /// their artwork writes and their document publication. + Future _queue = Future.value(); GeneratedDeckResultApplier({ required DeckDocumentStore documentStore, MemoryDeckLoader? deckLoader, @@ -26,19 +53,58 @@ final class GeneratedDeckResultApplier { _assetCacheStore = assetCacheStore, _customizationStore = customizationStore; - Future apply(DeckGenerationResult result) async { + /// Deletes only the artwork this attempt staged, and keeps every asset key + /// that an earlier attempt already committed. + Future _discardStagedAssets(Set stagedAssetKeys) async { + final cache = _assetCacheStore; + if (cache == null || stagedAssetKeys.isEmpty) return; + for (final assetKey in stagedAssetKeys) { + try { + await cache.delete(assetKey); + } catch (_) { + // An abandoned asset that cannot be deleted must not mask the reason + // the application stopped. + } + } + } + + Future _apply( + DeckGenerationResult result, + GeneratedDeckApplicationGuard isValid, + ) async { + const abandoned = GeneratedDeckApplication(published: false); + if (!isValid()) return abandoned; + + final cache = _assetCacheStore; final nextAssetKeys = {}; - for (final asset in result.generatedImages) { - final bytes = asset.bytes; - if (bytes == null || bytes.isEmpty) continue; - final cache = _assetCacheStore; - if (cache == null) { - throw StateError( - 'Generated artwork cannot be loaded without an asset cache.', - ); + final stagedAssetKeys = {}; + + try { + for (final asset in result.generatedImages) { + final bytes = asset.bytes; + if (bytes == null || bytes.isEmpty) continue; + if (cache == null) { + throw StateError( + 'Generated artwork cannot be loaded without an asset cache.', + ); + } + await cache.write(asset.assetKey, bytes); + nextAssetKeys.add(asset.assetKey); + if (!_appliedAssetKeys.contains(asset.assetKey)) { + stagedAssetKeys.add(asset.assetKey); + } } - await cache.write(asset.assetKey, bytes); - nextAssetKeys.add(asset.assetKey); + } catch (_) { + await _discardStagedAssets(stagedAssetKeys); + rethrow; + } + + // The asset writes above are asynchronous, so the host can cancel or + // supersede this generation while they run. + if (!isValid()) { + await _discardStagedAssets(stagedAssetKeys); + + return abandoned; } final markdown = const SlideSerializer().serialize(result.slides); @@ -48,12 +114,53 @@ final class GeneratedDeckResultApplier { _customizationStore.applyGeneratedStyle(theme.toGeneratedDeckStyle()); } + return GeneratedDeckApplication( + published: true, + cleanupError: await _removeObsoleteAssets(nextAssetKeys), + ); + } + + /// Deletes the artwork the newly published deck no longer references. + /// + /// Returns the first deletion failure, which the host reports separately + /// from a generation failure because the deck is already published. + Future _removeObsoleteAssets(Set nextAssetKeys) async { final cache = _assetCacheStore; - if (cache != null) { - for (final assetKey in _appliedAssetKeys.difference(nextAssetKeys)) { + final obsoleteAssetKeys = _appliedAssetKeys.difference(nextAssetKeys); + _appliedAssetKeys = Set.unmodifiable(nextAssetKeys); + if (cache == null) return null; + + Object? cleanupError; + for (final assetKey in obsoleteAssetKeys) { + try { await cache.delete(assetKey); + } catch (error) { + cleanupError ??= error; } } - _appliedAssetKeys = Set.unmodifiable(nextAssetKeys); + + return cleanupError; + } + + /// Publishes [result] unless [isValid] reports that newer work replaced it. + /// + /// Returns whether the deck reached the document, the preview, and the + /// theme. Artwork staged by an abandoned attempt is removed again. + Future apply( + DeckGenerationResult result, { + required GeneratedDeckApplicationGuard isValid, + }) { + final application = Completer(); + // The queue only sequences the work. Failures reach the caller through + // the completer, so one failed application cannot block the next one. + _queue = _queue.then((_) async { + try { + application.complete(await _apply(result, isValid)); + } catch (error, stackTrace) { + application.completeError(error, stackTrace); + } + }); + + return application.future; } } diff --git a/packages/playground/lib/features/ai/quick_agent/presentation/pages/generation_lab_page.dart b/packages/playground/lib/features/ai/quick_agent/presentation/pages/generation_lab_page.dart index d843dcb0..85ef4751 100644 --- a/packages/playground/lib/features/ai/quick_agent/presentation/pages/generation_lab_page.dart +++ b/packages/playground/lib/features/ai/quick_agent/presentation/pages/generation_lab_page.dart @@ -5,13 +5,11 @@ import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; import 'package:provider/provider.dart'; import 'package:superdeck/superdeck.dart'; -import 'package:superdeck_builder/superdeck_builder.dart'; import '../../../../../core/data/data_sources/memory_asset_cache_store.dart'; -import '../../../../../core/data/data_sources/memory_deck_loader.dart'; import '../../../../../core/domain/design/presentation_image_style_catalog.dart'; import '../../../../../core/domain/design/presentation_theme_catalog.dart'; -import '../../../../../core/domain/stores/deck_customization_store.dart'; +import '../../../../editor/domain/stores/deck_document_store.dart'; import '../../../image_generation/image_generator.dart'; import '../../core/engine/schemas/outline_schema.dart'; import '../../core/engine/services/deck_generation_request.dart'; @@ -19,7 +17,7 @@ import '../../core/engine/services/deck_generator_service.dart'; import '../../core/engine/services/generation_progress.dart'; import '../../core/engine/services/generation_trace.dart'; import '../../core/env_config.dart'; -import '../../domain/generated_deck_style_mapper.dart'; +import '../../domain/generated_deck_result_applier.dart'; /// Debug-only harness for iterating on planning, artwork, and composition /// without repeating the conversational Wizard intake. @@ -44,6 +42,12 @@ class _GenerationLabPageState extends State { final _compositionTraces = []; late final DeckGeneratorService? _service; + + /// The lab keeps its own document, so experiments never publish into the + /// editor's document or its file binding. + final _documentStore = DeckDocumentStore(markdown: ''); + + GeneratedDeckResultApplier? _resultApplier; _GenerationPreset _preset = _presets.first; GenerationProgress _progress = const GenerationProgress(GenerationPhase.idle); DeckPlan? _plan; @@ -73,6 +77,17 @@ class _GenerationLabPageState extends State { : null); } + /// Resolves the shared applier on first use, so a lab screen without an + /// application host still renders its configuration guidance. + GeneratedDeckResultApplier _applier() { + return _resultApplier ??= GeneratedDeckResultApplier( + documentStore: _documentStore, + deckLoader: context.read(), + assetCacheStore: context.read(), + customizationStore: context.read(), + ); + } + void _selectPreset(_GenerationPreset preset) { if (_runningStage != null || identical(_preset, preset)) return; setState(() { @@ -204,17 +219,7 @@ class _GenerationLabPageState extends State { } Future _applyResult(DeckGenerationResult result) async { - final cache = context.read(); - final customization = context.read(); - final deckLoader = context.read(); - for (final asset in result.generatedImages) { - final bytes = asset.bytes; - if (bytes != null && bytes.isNotEmpty) { - await cache.write(asset.assetKey, bytes); - } - } - customization.applyGeneratedStyle(result.theme!.toGeneratedDeckStyle()); - deckLoader.updateMarkdown(const SlideSerializer().serialize(result.slides)); + await _applier().apply(result, isValid: () => mounted && !_cancelled); } bool _finishCancelled(_GenerationLabStage stage, Duration duration) { @@ -259,6 +264,7 @@ class _GenerationLabPageState extends State { @override void dispose() { _cancelled = true; + _documentStore.dispose(); super.dispose(); } diff --git a/packages/playground/lib/features/ai/wizard/presentation/wizard_generation_controller.dart b/packages/playground/lib/features/ai/wizard/presentation/wizard_generation_controller.dart index 1fea4a80..fde0d939 100644 --- a/packages/playground/lib/features/ai/wizard/presentation/wizard_generation_controller.dart +++ b/packages/playground/lib/features/ai/wizard/presentation/wizard_generation_controller.dart @@ -8,6 +8,7 @@ import '../../quick_agent/core/engine/services/deck_generator_service.dart'; import '../../quick_agent/core/engine/services/deck_plan_validator.dart'; import '../../quick_agent/core/engine/services/generation_progress.dart'; import '../../quick_agent/core/engine/services/generation_validation_issue.dart'; +import '../../quick_agent/domain/generated_deck_result_applier.dart'; enum WizardGenerationStage { setup, @@ -21,7 +22,10 @@ enum WizardGenerationStage { enum WizardGenerationPhase { planning, composition } typedef ApplyWizardDeckResult = - FutureOr Function(DeckGenerationResult result); + Future Function( + DeckGenerationResult result, { + required GeneratedDeckApplicationGuard isValid, + }); /// Owns the deterministic plan → review → compose lifecycle for the Wizard. final class WizardGenerationController extends ChangeNotifier { @@ -42,6 +46,7 @@ final class WizardGenerationController extends ChangeNotifier { DeckPlan? _plan; DeckGenerationResult? _result; String? _errorMessage; + String? _applyNotice; GenerationProgress _progress = const GenerationProgress(.idle); Duration _elapsed = .zero; DateTime? _stageStartedAt; @@ -71,6 +76,7 @@ final class WizardGenerationController extends ChangeNotifier { _result = null; _errorMessage = null; _failedPhase = null; + _applyNotice = null; _cancelled = false; _beginStage(.planning); _progress = const GenerationProgress(.generatingOutline); @@ -84,13 +90,13 @@ final class WizardGenerationController extends ChangeNotifier { isCancelled: () => _isOperationCancelled(operation), ); } catch (error) { - _finishTiming(); + _finishTiming(operation); if (!_isCurrentOperation(operation)) return; _fail(.planning, 'Could not create the outline: $error'); return; } - _finishTiming(); + _finishTiming(operation); if (!_isCurrentOperation(operation) || _disposed) return; if (_cancelled) return; if (!planning.success || planning.plan == null) { @@ -111,7 +117,16 @@ final class WizardGenerationController extends ChangeNotifier { _stageStartedAt = DateTime.now(); } - void _finishTiming() { + /// Records elapsed time only for the operation that still owns the stage. + /// + /// A superseded run must not add its duration to, or clear the start of, + /// the run that replaced it. + void _finishTiming(int operation) { + if (!_isCurrentOperation(operation)) return; + _finishStageTiming(); + } + + void _finishStageTiming() { final startedAt = _stageStartedAt; if (startedAt != null) { final duration = DateTime.now().difference(startedAt); @@ -141,8 +156,12 @@ final class WizardGenerationController extends ChangeNotifier { int operation, DeckGenerationResult generated, ) async { + final GeneratedDeckApplication application; try { - await _applyResult(generated); + application = await _applyResult( + generated, + isValid: () => !_isOperationCancelled(operation) && !_disposed, + ); } catch (error) { if (_isOperationCancelled(operation) || _disposed) return; _fail( @@ -153,6 +172,13 @@ final class WizardGenerationController extends ChangeNotifier { return; } if (_isOperationCancelled(operation) || _disposed) return; + // The applier stops when a newer run replaced this one, so the run that + // owns the state keeps it. + if (!application.published) return; + if (application.cleanupError != null) { + _applyNotice = + 'Some artwork from the previous deck could not be removed.'; + } _result = generated; _stage = .completed; _progress = const GenerationProgress(.idle); @@ -175,6 +201,12 @@ final class WizardGenerationController extends ChangeNotifier { String? get errorMessage => _errorMessage; + /// Non-blocking detail about applying the last published deck. + /// + /// Publication succeeded when this is set; only the cleanup of the replaced + /// deck's artwork did not. + String? get applyNotice => _applyNotice; + GenerationProgress get progress => _progress; Duration get elapsed => @@ -246,6 +278,7 @@ final class WizardGenerationController extends ChangeNotifier { _result = null; _errorMessage = null; _failedPhase = null; + _applyNotice = null; _cancelled = false; _beginStage(.composing); _progress = const GenerationProgress(.composingSlides); @@ -260,13 +293,13 @@ final class WizardGenerationController extends ChangeNotifier { isCancelled: () => _isOperationCancelled(operation), ); } catch (error) { - _finishTiming(); + _finishTiming(operation); if (!_isCurrentOperation(operation)) return; _fail(.composition, 'Could not compose the slides: $error'); return; } - _finishTiming(); + _finishTiming(operation); if (!_isCurrentOperation(operation) || _disposed || _cancelled) return; if ((!generated.success && !generated.isPartial) || generated.slides.isEmpty) { @@ -292,6 +325,7 @@ final class WizardGenerationController extends ChangeNotifier { final operation = ++_operationEpoch; _errorMessage = null; _failedPhase = null; + _applyNotice = null; _cancelled = false; _beginStage(.composing); _progress = const GenerationProgress(.composingSlides); @@ -306,13 +340,13 @@ final class WizardGenerationController extends ChangeNotifier { isCancelled: () => _isOperationCancelled(operation), ); } catch (error) { - _finishTiming(); + _finishTiming(operation); if (!_isCurrentOperation(operation)) return; _fail(.composition, 'Could not retry the unresolved slides: $error'); return; } - _finishTiming(); + _finishTiming(operation); if (!_isCurrentOperation(operation) || _disposed || _cancelled) return; if ((!generated.success && !generated.isPartial) || generated.slides.isEmpty) { @@ -368,6 +402,7 @@ final class WizardGenerationController extends ChangeNotifier { _planRevision = 0; _result = null; _errorMessage = null; + _applyNotice = null; _failedPhase = null; _progress = const GenerationProgress(.idle); _elapsed = .zero; @@ -380,7 +415,7 @@ final class WizardGenerationController extends ChangeNotifier { if (!isBusy || _cancelled) return; _cancelled = true; _operationEpoch++; - _finishTiming(); + _finishStageTiming(); _stage = _result?.isPartial == true ? .completed : _plan == null diff --git a/packages/playground/lib/features/ai/wizard/presentation/wizard_page.dart b/packages/playground/lib/features/ai/wizard/presentation/wizard_page.dart index 380a3f36..8e4c2c88 100644 --- a/packages/playground/lib/features/ai/wizard/presentation/wizard_page.dart +++ b/packages/playground/lib/features/ai/wizard/presentation/wizard_page.dart @@ -177,7 +177,10 @@ class _WizardExperience extends StatelessWidget { .completed => _CenteredScrollable( child: WizardGenerationStatus( kind: .completed, - noticeMessage: _completionNotice(controller.result), + noticeMessage: _completionNotice( + controller.result, + controller.applyNotice, + ), slideCount: controller.result?.slides.length, failedSlideCount: controller.result?.slideFailures.length ?? 0, artworkCount: controller.result?.generatedImageCount ?? 0, @@ -241,9 +244,9 @@ class _WizardExperience extends StatelessWidget { } } -String? _completionNotice(DeckGenerationResult? result) { +String? _completionNotice(DeckGenerationResult? result, String? applyNotice) { if (result == null) return null; - final messages = []; + final messages = [?applyNotice]; if (result.isPartial) { final failed = result.slideFailures.length; messages.add( diff --git a/packages/playground/lib/features/editor/domain/stores/deck_document_store.dart b/packages/playground/lib/features/editor/domain/stores/deck_document_store.dart index a346503b..e8359142 100644 --- a/packages/playground/lib/features/editor/domain/stores/deck_document_store.dart +++ b/packages/playground/lib/features/editor/domain/stores/deck_document_store.dart @@ -7,6 +7,8 @@ import 'package:flutter/foundation.dart'; /// intentionally ignored so attribution-only editor changes cannot trigger a /// persistence or preview feedback loop. class DeckDocumentStore extends ChangeNotifier { + int _revision = 0; + DeckDocumentStore({required String markdown}) : _markdown = markdown; String _markdown; @@ -14,10 +16,17 @@ class DeckDocumentStore extends ChangeNotifier { /// The current full Markdown document. String get markdown => _markdown; + /// Counts the accepted replacements of this document. + /// + /// A long-running operation captures this value when it starts and compares + /// it before it publishes, so it cannot overwrite a newer document. + int get revision => _revision; + /// Replaces the document and notifies listeners when its text changed. void replaceMarkdown(String markdown) { if (markdown == _markdown) return; _markdown = markdown; + _revision++; notifyListeners(); } } diff --git a/packages/playground/lib/features/editor/domain/stores/deck_file_session.dart b/packages/playground/lib/features/editor/domain/stores/deck_file_session.dart index 9f136cc2..3215353c 100644 --- a/packages/playground/lib/features/editor/domain/stores/deck_file_session.dart +++ b/packages/playground/lib/features/editor/domain/stores/deck_file_session.dart @@ -78,6 +78,13 @@ class DeckFileSession extends ChangeNotifier { /// Current persistence binding state. DeckBindingStatus get status => _status; + /// Counts the changes of the file this document is bound to. + /// + /// The value changes when the session binds another deck and when the bound + /// file is lost. It therefore separates two decks that hold identical + /// content, which the document revision alone cannot do. + int get bindingRevision => _bindingEpoch; + /// Whether local document changes are currently auto-saved. bool get isBound => _status == DeckBindingStatus.bound; diff --git a/packages/playground/lib/features/editor/domain/stores/editor_store.dart b/packages/playground/lib/features/editor/domain/stores/editor_store.dart index 0978fb03..a72107e6 100644 --- a/packages/playground/lib/features/editor/domain/stores/editor_store.dart +++ b/packages/playground/lib/features/editor/domain/stores/editor_store.dart @@ -2,10 +2,10 @@ import 'package:flutter/foundation.dart'; /// Shared editor navigation state: which slide the caret currently sits in. /// -/// Pure domain state with no super_editor coupling. The document itself lives in -/// the presentation-layer `TextEditorController`, which keeps this in sync with -/// the caret and reacts when it's set from outside the editor (e.g. a preview -/// tap) by scrolling the caret to that slide. +/// Pure domain state with no super_editor coupling. `DeckDocumentStore` owns +/// the document. The presentation-layer `TextEditorController` keeps this store +/// in sync with the caret and scrolls to the selected slide when a preview tap +/// changes the active index. class EditorStore extends ChangeNotifier { static const double minPreviewSidebarWidth = 160; static const double maxPreviewSidebarWidth = 480; diff --git a/packages/playground/lib/features/editor/presentation/pages/editor_bootstrap.dart b/packages/playground/lib/features/editor/presentation/pages/editor_bootstrap.dart index fab0f9e5..79b6b422 100644 --- a/packages/playground/lib/features/editor/presentation/pages/editor_bootstrap.dart +++ b/packages/playground/lib/features/editor/presentation/pages/editor_bootstrap.dart @@ -85,6 +85,7 @@ class _EditorBootstrapState extends State { customizationStore: ctx.read(), deckLoader: ctx.read(), assetCacheStore: ctx.read(), + bindingRevision: () => fileSession.bindingRevision, ), dispose: (_, command) => command.dispose(), ), diff --git a/packages/playground/lib/features/editor/presentation/widgets/editor_header.dart b/packages/playground/lib/features/editor/presentation/widgets/editor_header.dart index d7114013..655a1f2c 100644 --- a/packages/playground/lib/features/editor/presentation/widgets/editor_header.dart +++ b/packages/playground/lib/features/editor/presentation/widgets/editor_header.dart @@ -3,12 +3,16 @@ import 'package:hero_ui/hero_ui.dart'; import 'package:mix/mix.dart'; import 'package:provider/provider.dart'; +import '../../../ai/quick_agent/domain/commands/generate_deck_command.dart'; import '../../domain/stores/deck_file_session.dart'; import 'new_deck_dialog.dart'; /// Bar sitting on top of the text editor: the `New` / `Open` actions on the /// left, followed by the bound deck's filename. When the bound file is lost /// (deleted/moved) it also surfaces the controller's warning as a banner. +/// +/// Generation reports its own non-blocking notice here, because the panel that +/// started the run can be closed before the deck arrives. class EditorHeader extends StatelessWidget { const EditorHeader({super.key}); @@ -16,6 +20,8 @@ class EditorHeader extends StatelessWidget { Widget build(BuildContext context) { final session = context.watch(); final warning = session.warning; + final generation = context.watch(); + final notice = generation.completionNotice; return ColumnBox( style: FlexBoxStyler().mainAxisSize(.min), @@ -53,6 +59,8 @@ class EditorHeader extends StatelessWidget { ), ), if (warning != null) _WarningBanner(message: warning), + if (notice != null) + _NoticeBanner(message: notice, onDismiss: generation.dismissNotice), ], ); } @@ -115,3 +123,48 @@ class _WarningBanner extends StatelessWidget { ); } } + +class _NoticeBanner extends StatelessWidget { + const _NoticeBanner({required this.message, required this.onDismiss}); + + final String message; + final VoidCallback onDismiss; + + @override + Widget build(BuildContext context) { + return Box( + style: BoxStyler() + .width(double.infinity) + .color($accent().withValues(alpha: 0.12)) + .padding(.horizontal(16).vertical(8)), + child: Row( + spacing: 8, + children: [ + Icon( + CupertinoIcons.sparkles, + size: 14, + color: $accent.resolve(context), + ), + Expanded( + child: StyledText( + message, + style: TextStyler().color($accent()).style($labelSmall.mix()), + ), + ), + SizedBox( + width: 28, + child: Semantics( + label: 'Dismiss generation notice', + child: HeroIconButton( + variant: .ghost, + size: .sm, + icon: CupertinoIcons.xmark, + onPressed: onDismiss, + ), + ), + ), + ], + ), + ); + } +} diff --git a/packages/playground/test/features/ai/quick_agent/domain/commands/generate_deck_command_test.dart b/packages/playground/test/features/ai/quick_agent/domain/commands/generate_deck_command_test.dart index 7960964a..0ebffc71 100644 --- a/packages/playground/test/features/ai/quick_agent/domain/commands/generate_deck_command_test.dart +++ b/packages/playground/test/features/ai/quick_agent/domain/commands/generate_deck_command_test.dart @@ -1,3 +1,4 @@ +import 'package:flutter/foundation.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:google_fonts/google_fonts.dart'; import 'package:playground/core/data/data_sources/memory_deck_loader.dart'; @@ -75,22 +76,145 @@ void main() { expect(result, isA>()); expect(documentStore.markdown, contains('Accepted slide')); expect(command.completionNotice, contains('Generated 1 of 2 slides')); + + // The panel clears the command result when it closes. The editor keeps + // showing the notice until it is dismissed. + command.clearResult(); + expect(command.completionNotice, contains('Generated 1 of 2 slides')); + command.dismissNotice(); + expect(command.completionNotice, isNull); + }, + ); + + test( + 'discards the deck when the document changed while generating', + () async { + final host = _CommandHost(); + addTearDown(host.dispose); + final command = host.command( + onGenerate: () => host.documentStore.replaceMarkdown('# Manual edit'), + ); + + final result = await command.action( + const DeckGenerationRequest(userIntent: 'Test deck', slideCount: 1), + ); + + expect(result, isA>()); + expect(host.documentStore.markdown, '# Manual edit'); + expect(command.completionNotice, contains('discarded')); + }, + ); + + test( + 'discards the deck when the bound deck changed while generating', + () async { + final host = _CommandHost(); + addTearDown(host.dispose); + // A deck switch to a file with identical content leaves the document + // revision unchanged, so only the binding revision reports it. + final command = host.command(onGenerate: () => host.bindingRevision++); + + final result = await command.action( + const DeckGenerationRequest(userIntent: 'Test deck', slideCount: 1), + ); + + expect(result, isA>()); + expect(host.documentStore.markdown, isEmpty); + expect(command.completionNotice, contains('discarded')); }, ); + + test('publishes the deck when nothing replaced it', () async { + final host = _CommandHost(); + addTearDown(host.dispose); + final command = host.command(); + + final result = await command.action( + const DeckGenerationRequest(userIntent: 'Test deck', slideCount: 1), + ); + + expect(result, isA>()); + expect(host.documentStore.markdown, contains('Accepted slide')); + expect(command.completionNotice, isNull); + }); +} + +/// Owns the stores one [GenerateDeckCommand] writes into. +final class _CommandHost { + _CommandHost() { + _controller = DeckController( + deckLoader: MemoryDeckLoader(), + options: DeckOptions(), + ); + _customizationStore = DeckCustomizationStore(_controller); + } + + final documentStore = DeckDocumentStore(markdown: ''); + final _commands = []; + late final DeckController _controller; + late final DeckCustomizationStore _customizationStore; + int bindingRevision = 0; + + GenerateDeckCommand command({VoidCallback? onGenerate}) { + final command = GenerateDeckCommand( + documentStore: documentStore, + customizationStore: _customizationStore, + bindingRevision: () => bindingRevision, + service: _StubDeckGeneratorService( + _completeResult(), + onGenerate: onGenerate, + ), + ); + _commands.add(command); + + return command; + } + + void dispose() { + for (final command in _commands) { + command.dispose(); + } + _customizationStore.dispose(); + _controller.dispose(); + documentStore.dispose(); + } } +DeckGenerationResult _completeResult() => DeckGenerationResult.success( + slides: [ + Slide( + key: 'accepted', + options: SlideOptions(title: 'Accepted slide', style: 'content'), + sections: [ + SectionBlock.text('## Accepted slide\n\nUseful grounded content.'), + ], + ), + ], + plan: _plan(), + theme: _resolvedTheme(), +); + final class _StubDeckGeneratorService extends DeckGeneratorService { - _StubDeckGeneratorService(this.result) : super(apiKey: 'test-key'); + _StubDeckGeneratorService(this.result, {this.onGenerate}) + : super(apiKey: 'test-key'); final DeckGenerationResult result; + /// Runs while the deck is being produced, so a test can replace the + /// document or switch decks mid-run. + final VoidCallback? onGenerate; + @override Future generate( DeckGenerationRequest request, { GenerationProgressCallback? onProgress, GenerationTraceCallback? onTrace, bool Function()? isCancelled, - }) async => result; + }) async { + onGenerate?.call(); + + return result; + } } DeckPlan _plan() => DeckPlan.parse({ diff --git a/packages/playground/test/features/ai/quick_agent/domain/generated_deck_result_applier_test.dart b/packages/playground/test/features/ai/quick_agent/domain/generated_deck_result_applier_test.dart index 345fbd14..f8badd8c 100644 --- a/packages/playground/test/features/ai/quick_agent/domain/generated_deck_result_applier_test.dart +++ b/packages/playground/test/features/ai/quick_agent/domain/generated_deck_result_applier_test.dart @@ -1,3 +1,6 @@ +import 'dart:async'; + +import 'package:flutter/material.dart' show Color; import 'package:flutter_test/flutter_test.dart'; import 'package:google_fonts/google_fonts.dart'; import 'package:playground/core/data/data_sources/memory_asset_cache_store.dart'; @@ -11,6 +14,7 @@ import 'package:playground/features/ai/quick_agent/core/engine/services/deck_gen import 'package:playground/features/ai/quick_agent/domain/generated_deck_result_applier.dart'; import 'package:playground/features/editor/domain/stores/deck_document_store.dart'; import 'package:superdeck/superdeck.dart'; +import 'package:superdeck_builder/superdeck_builder.dart'; import 'package:superdeck_core/superdeck_core.dart'; void main() { @@ -44,7 +48,7 @@ void main() { assetCacheStore: cache, customizationStore: customizationStore, ); - await applier.apply( + final application = await applier.apply( DeckGenerationResult.success( slides: [_generatedSlide(assetKey)], plan: _plan(assetKey), @@ -53,8 +57,11 @@ void main() { GeneratedImageAsset.success(assetKey: assetKey, bytes: [1, 2, 3]), ], ), + isValid: _always, ); + expect(application.published, isTrue); + expect(application.cleanupError, isNull); expect(await published, isNotNull); expect(documentStore.markdown, contains(assetKey)); }, @@ -83,21 +90,316 @@ void main() { addTearDown(loader.dispose); addTearDown(documentStore.dispose); - await applier.apply(_result(oldAssetKey)); + await applier.apply(_result(oldAssetKey), isValid: _always); expect(await cache.resolve(oldAssetKey), isNotNull); - await applier.apply(_result(nextAssetKey)); + await applier.apply(_result(nextAssetKey), isValid: _always); expect(await cache.resolve(oldAssetKey), isNull); expect(await cache.resolve(nextAssetKey), isNotNull); }); + + test('removes artwork staged by an invalidated application', () async { + const committedAssetKey = 'wizard-committed-slide-01-opening.png'; + const stagedAssetKey = 'wizard-staged-slide-01-opening.png'; + final cache = _BlockingAssetCacheStore(); + final host = _ApplierHost(cache); + addTearDown(host.dispose); + + await host.applier.apply(_result(committedAssetKey), isValid: _always); + expect(await cache.resolve(committedAssetKey), isNotNull); + + final publishedMarkdown = host.documentStore.markdown; + final publishedTheme = host.appliedTheme; + final previewEvents = []; + final subscription = host._loader.load().listen(previewEvents.add); + addTearDown(subscription.cancel); + var valid = true; + cache.blockNextWrite = true; + final application = host.applier.apply( + _result(stagedAssetKey, themeId: 'bold-product'), + isValid: () => valid, + ); + await cache.writeStarted.future; + // The document moves on while the artwork write is still in flight. + valid = false; + cache.releaseWrite(); + + expect((await application).published, isFalse); + expect(await cache.resolve(stagedAssetKey), isNull); + expect(await cache.resolve(committedAssetKey), isNotNull); + expect(host.documentStore.markdown, publishedMarkdown); + expect(host.appliedTheme, publishedTheme); + expect(previewEvents, isEmpty); + }); + + test( + 'a write failure cleans staged assets and allows a later application', + () async { + const committedKey = 'wizard-committed.png'; + const stagedKey = 'wizard-staged.png'; + const failedKey = 'wizard-failed.png'; + const nextKey = 'wizard-next.png'; + final cache = _BlockingAssetCacheStore(); + final host = _ApplierHost(cache); + addTearDown(host.dispose); + + await host.applier.apply(_result(committedKey), isValid: _always); + final publishedMarkdown = host.documentStore.markdown; + final publishedTheme = host.appliedTheme; + final previewEvents = []; + final subscription = host._loader.load().listen(previewEvents.add); + addTearDown(subscription.cancel); + cache.failWriteFor = failedKey; + + await expectLater( + host.applier.apply( + _result( + stagedKey, + themeId: 'bold-product', + additionalImages: [ + GeneratedImageAsset.success( + assetKey: failedKey, + bytes: [4, 5, 6], + ), + ], + ), + isValid: _always, + ), + throwsA( + isA().having( + (error) => error.message, + 'message', + contains(failedKey), + ), + ), + ); + + expect(host.documentStore.markdown, publishedMarkdown); + expect(host.appliedTheme, publishedTheme); + expect(previewEvents, isEmpty); + expect(await cache.resolve(committedKey), isNotNull); + expect(await cache.resolve(stagedKey), isNull); + expect(await cache.resolve(failedKey), isNull); + + final application = await host.applier.apply( + _result(nextKey, themeId: 'bold-product'), + isValid: _always, + ); + + expect(application.published, isTrue); + expect(host.documentStore.markdown, contains(nextKey)); + expect(host.appliedTheme, isNot(publishedTheme)); + expect(previewEvents.whereType(), hasLength(1)); + expect(await cache.resolve(nextKey), isNotNull); + expect(await cache.resolve(committedKey), isNull); + }, + ); + + test('keeps the committed deck when its own artwork is reused', () async { + const assetKey = 'wizard-reused-slide-01-opening.png'; + final cache = _BlockingAssetCacheStore(); + final host = _ApplierHost(cache); + addTearDown(host.dispose); + + await host.applier.apply(_result(assetKey), isValid: _always); + + var valid = true; + cache.blockNextWrite = true; + final application = host.applier.apply( + _result(assetKey), + isValid: () => valid, + ); + await cache.writeStarted.future; + valid = false; + cache.releaseWrite(); + + expect((await application).published, isFalse); + expect(await cache.resolve(assetKey), isNotNull); + }); + + test('reports obsolete-asset cleanup failures after publishing', () async { + const oldAssetKey = 'wizard-old-slide-01-opening.png'; + const nextAssetKey = 'wizard-next-slide-01-opening.png'; + final cache = _BlockingAssetCacheStore(); + final host = _ApplierHost(cache); + addTearDown(host.dispose); + + await host.applier.apply(_result(oldAssetKey), isValid: _always); + + cache.failDeleteFor = oldAssetKey; + final application = await host.applier.apply( + _result(nextAssetKey), + isValid: _always, + ); + + expect(application.published, isTrue); + expect(application.cleanupError, isNotNull); + expect(host.documentStore.markdown, contains(nextAssetKey)); + expect(await cache.resolve(nextAssetKey), isNotNull); + }); + + test('applies queued results one at a time', () async { + const firstAssetKey = 'wizard-first-slide-01-opening.png'; + const secondAssetKey = 'wizard-second-slide-01-opening.png'; + final cache = _BlockingAssetCacheStore(); + final host = _ApplierHost(cache); + addTearDown(host.dispose); + + cache.blockNextWrite = true; + final first = host.applier.apply(_result(firstAssetKey), isValid: _always); + await cache.writeStarted.future; + final second = host.applier.apply( + _result(secondAssetKey), + isValid: _always, + ); + + // The second application cannot start while the first one is writing. + await Future.delayed(Duration.zero); + expect(cache.writes, [firstAssetKey]); + + cache.releaseWrite(); + expect((await first).published, isTrue); + expect((await second).published, isTrue); + expect(cache.writes, [firstAssetKey, secondAssetKey]); + expect(host.documentStore.markdown, contains(secondAssetKey)); + expect(await cache.resolve(firstAssetKey), isNull); + }); + + test( + 'publishes the document, preview, theme, and artwork together', + () async { + const assetKey = 'wizard-published-slide-01-opening.png'; + final cache = _BlockingAssetCacheStore(); + final host = _ApplierHost(cache); + addTearDown(host.dispose); + + final previewMarkdown = host.previewMarkdown; + final themeBefore = host.appliedTheme; + + final application = await host.applier.apply( + _result(assetKey), + isValid: _always, + ); + + expect(application.published, isTrue); + expect(host.documentStore.markdown, contains(assetKey)); + expect(await previewMarkdown, contains(assetKey)); + expect(host.appliedTheme, isNot(themeBefore)); + expect(host.appliedTheme.headlineFamily, 'Space Grotesk'); + expect(await cache.resolve(assetKey), isNotNull); + }, + ); +} + +bool _always() => true; + +/// Owns the stores one applier writes into. +final class _ApplierHost { + _ApplierHost(AssetCacheStore cache) + : documentStore = DeckDocumentStore(markdown: ''), + _loader = MemoryDeckLoader(), + _cache = cache { + _deckController = DeckController( + deckLoader: _loader, + options: DeckOptions(), + assetCacheStore: _cache, + ); + customizationStore = DeckCustomizationStore(_deckController); + applier = GeneratedDeckResultApplier( + documentStore: documentStore, + deckLoader: _loader, + assetCacheStore: _cache, + customizationStore: customizationStore, + ); + } + + final DeckDocumentStore documentStore; + final MemoryDeckLoader _loader; + final AssetCacheStore _cache; + late final DeckController _deckController; + late final DeckCustomizationStore customizationStore; + late final GeneratedDeckResultApplier applier; + + /// The theme state a published result must change, and an abandoned one + /// must leave alone. + ({Color background, String headlineFamily}) get appliedTheme => ( + background: customizationStore.background, + headlineFamily: customizationStore.level(TextLevel.h1).family, + ); + + /// The markdown the preview loader receives next. + Future get previewMarkdown => _loader + .load() + .where((event) => event is SlidesLoadedEvent) + .cast() + .first + .then((event) => const SlideSerializer().serialize(event.slides)); + + void dispose() { + customizationStore.dispose(); + _deckController.dispose(); + unawaited(_loader.dispose()); + documentStore.dispose(); + } +} + +/// Asset cache that can hold one write open and fail selected writes or deletes. +final class _BlockingAssetCacheStore implements AssetCacheStore { + final _store = MemoryAssetCacheStore(); + final writes = []; + + var writeStarted = Completer(); + Completer? _writeGate; + bool blockNextWrite = false; + String? failWriteFor; + String? failDeleteFor; + + void releaseWrite() { + _writeGate?.complete(); + _writeGate = null; + } + + @override + Future resolve(String assetKey) => _store.resolve(assetKey); + + @override + Future write(String assetKey, List bytes) async { + writes.add(assetKey); + if (assetKey == failWriteFor) { + throw StateError('Cannot write $assetKey.'); + } + if (blockNextWrite) { + blockNextWrite = false; + final gate = _writeGate = Completer(); + if (!writeStarted.isCompleted) writeStarted.complete(); + await gate.future; + writeStarted = Completer(); + } + + return _store.write(assetKey, bytes); + } + + @override + Future delete(String assetKey) async { + if (assetKey == failDeleteFor) { + throw StateError('Cannot delete $assetKey.'); + } + + return _store.delete(assetKey); + } } -DeckGenerationResult _result(String assetKey) => DeckGenerationResult.success( +DeckGenerationResult _result( + String assetKey, { + String themeId = 'technical-paper', + List additionalImages = const [], +}) => DeckGenerationResult.success( slides: [_generatedSlide(assetKey)], - plan: _plan(assetKey), - theme: _resolvedTheme(), + plan: _plan(assetKey, themeId: themeId), + theme: _resolvedTheme(themeId), generatedImages: [ GeneratedImageAsset.success(assetKey: assetKey, bytes: [1, 2, 3]), + ...additionalImages, ], ); @@ -118,44 +420,50 @@ Slide _generatedSlide(String assetKey) => Slide.parse({ ], }); -DeckPlan _plan(String assetKey) => DeckPlan.parse({ - 'topic': 'Generated artwork', - 'story': 'One image supports one clear point.', - 'theme': {'id': 'technical-paper', 'version': 1, 'density': 'balanced'}, - 'sections': [ - { - 'key': 'main', - 'title': 'Main', - 'purpose': 'Introduce the idea.', - 'transition': 'Close clearly.', - 'slideKeys': ['opening'], - }, - ], - 'slides': [ - { - 'key': 'opening', - 'title': 'Opening', - 'purpose': 'Introduce the idea.', - 'sectionKey': 'main', - 'assertion': 'The visual makes the idea tangible.', - 'contentUnits': ['One focused supporting statement.'], - 'narrativeRole': 'opening', - 'contentBrief': 'Open with one clear idea.', - 'continuity': 'Lead into the story.', - 'composition': 'imageFullBleed', - 'treatment': 'visual', - 'density': 'balanced', - 'elements': [ - {'type': 'image', 'purpose': 'Anchor the story.', 'source': assetKey}, +DeckPlan _plan(String assetKey, {String themeId = 'technical-paper'}) => + DeckPlan.parse({ + 'topic': 'Generated artwork', + 'story': 'One image supports one clear point.', + 'theme': {'id': themeId, 'version': 1, 'density': 'balanced'}, + 'sections': [ + { + 'key': 'main', + 'title': 'Main', + 'purpose': 'Introduce the idea.', + 'transition': 'Close clearly.', + 'slideKeys': ['opening'], + }, ], - }, - ], -}); + 'slides': [ + { + 'key': 'opening', + 'title': 'Opening', + 'purpose': 'Introduce the idea.', + 'sectionKey': 'main', + 'assertion': 'The visual makes the idea tangible.', + 'contentUnits': ['One focused supporting statement.'], + 'narrativeRole': 'opening', + 'contentBrief': 'Open with one clear idea.', + 'continuity': 'Lead into the story.', + 'composition': 'imageFullBleed', + 'treatment': 'visual', + 'density': 'balanced', + 'elements': [ + { + 'type': 'image', + 'purpose': 'Anchor the story.', + 'source': assetKey, + }, + ], + }, + ], + }); -ResolvedPresentationTheme _resolvedTheme() => - PresentationThemeCatalog.withDefaults().resolve( - id: 'technical-paper', - version: 1, - density: 'balanced', - typographyCatalog: PresentationTypographyCatalog.withDefaults(), - ); +ResolvedPresentationTheme _resolvedTheme([ + String themeId = 'technical-paper', +]) => PresentationThemeCatalog.withDefaults().resolve( + id: themeId, + version: 1, + density: 'balanced', + typographyCatalog: PresentationTypographyCatalog.withDefaults(), +); diff --git a/packages/playground/test/features/ai/wizard/presentation/wizard_generation_controller_test.dart b/packages/playground/test/features/ai/wizard/presentation/wizard_generation_controller_test.dart index 5b593033..8f1459b5 100644 --- a/packages/playground/test/features/ai/wizard/presentation/wizard_generation_controller_test.dart +++ b/packages/playground/test/features/ai/wizard/presentation/wizard_generation_controller_test.dart @@ -8,6 +8,7 @@ import 'package:playground/features/ai/quick_agent/core/engine/services/deck_gen import 'package:playground/features/ai/quick_agent/core/engine/services/deck_generator_service.dart'; import 'package:playground/features/ai/quick_agent/core/engine/services/deck_theme_resolution.dart'; import 'package:playground/features/ai/quick_agent/core/engine/services/generation_validation_issue.dart'; +import 'package:playground/features/ai/quick_agent/domain/generated_deck_result_applier.dart'; import 'package:playground/features/ai/wizard/presentation/wizard_generation_controller.dart'; import 'package:superdeck_core/superdeck_core.dart'; @@ -19,10 +20,10 @@ void main() { themeId: 'technical-paper', ); final service = _FakeWizardGenerationService(_plan(request)); - DeckGenerationResult? appliedResult; + final applier = _RecordingApplier(); final controller = WizardGenerationController( service: service, - applyResult: (result) => appliedResult = result, + applyResult: applier.apply, ); addTearDown(controller.dispose); @@ -51,8 +52,9 @@ void main() { service.approvedPlan!.slides.single.assertion, 'Small urban gardens create city-scale resilience.', ); - expect(appliedResult, isNotNull); - expect(controller.result, same(appliedResult)); + expect(applier.applied, hasLength(1)); + expect(controller.result, same(applier.applied.single)); + expect(controller.applyNotice, isNull); }); test( @@ -69,7 +71,7 @@ void main() { ); final controller = WizardGenerationController( service: service, - applyResult: (_) {}, + applyResult: _RecordingApplier().apply, ); addTearDown(controller.dispose); @@ -96,10 +98,10 @@ void main() { _plan(request, slideKeys: const ['opening', 'evidence']), partialComposition: true, ); - final applied = []; + final applier = _RecordingApplier(); final controller = WizardGenerationController( service: service, - applyResult: applied.add, + applyResult: applier.apply, ); addTearDown(controller.dispose); @@ -119,7 +121,7 @@ void main() { 'evidence', ]); expect(service.retryCalls, 1); - expect(applied, hasLength(2)); + expect(applier.applied, hasLength(2)); }); test('cancels immediately and ignores the late planning result', () async { @@ -133,7 +135,7 @@ void main() { ..pendingPlanning = pending; final controller = WizardGenerationController( service: service, - applyResult: (_) {}, + applyResult: _RecordingApplier().apply, ); addTearDown(controller.dispose); @@ -158,13 +160,12 @@ void main() { final service = _FakeWizardGenerationService(_plan(request)); final applicationStarted = Completer(); final pendingApplication = Completer(); + final applier = _RecordingApplier() + ..started = applicationStarted + ..pending = pendingApplication; final controller = WizardGenerationController( service: service, - applyResult: (_) { - applicationStarted.complete(); - - return pendingApplication.future; - }, + applyResult: applier.apply, ); addTearDown(controller.dispose); @@ -180,6 +181,73 @@ void main() { expect(controller.stage, WizardGenerationStage.outlineReview); expect(controller.result, isNull); + expect(applier.applied, isEmpty); + }); + + test('reports a cleanup failure without failing the generation', () async { + const request = DeckGenerationRequest( + userIntent: 'Urban gardens', + slideCount: 1, + themeId: 'technical-paper', + ); + final service = _FakeWizardGenerationService(_plan(request)); + final applier = _RecordingApplier() + ..cleanupError = StateError('artwork is locked'); + final controller = WizardGenerationController( + service: service, + applyResult: applier.apply, + ); + addTearDown(controller.dispose); + + await controller.createOutline(request); + await controller.generateSlides(); + + expect(controller.stage, WizardGenerationStage.completed); + expect(controller.errorMessage, isNull); + expect(controller.result, isNotNull); + expect(controller.applyNotice, contains('could not be removed')); + }); + + test('a superseded run does not stop the current run clock', () async { + const request = DeckGenerationRequest( + userIntent: 'Urban gardens', + slideCount: 1, + themeId: 'technical-paper', + ); + final superseded = Completer(); + final current = Completer(); + final service = _FakeWizardGenerationService(_plan(request)) + ..pendingPlanning = superseded; + final controller = WizardGenerationController( + service: service, + applyResult: _RecordingApplier().apply, + ); + addTearDown(controller.dispose); + + final first = controller.createOutline(request); + await Future.delayed(const Duration(milliseconds: 10)); + controller.cancel(); + + service.pendingPlanning = current; + final second = controller.createOutline(request); + await Future.delayed(const Duration(milliseconds: 10)); + expect(controller.stage, WizardGenerationStage.planning); + + // The superseded run finishes while the current run is still planning. + superseded.complete(DeckPlanningResult.success(_plan(request))); + await first; + + final before = controller.elapsed; + await Future.delayed(const Duration(milliseconds: 30)); + final after = controller.elapsed; + expect( + after - before, + greaterThanOrEqualTo(const Duration(milliseconds: 20)), + ); + + current.complete(DeckPlanningResult.success(_plan(request))); + await second; + expect(controller.stage, WizardGenerationStage.outlineReview); }); testWidgets('keeps composing after the 30-second performance target', ( @@ -195,7 +263,7 @@ void main() { ..pendingComposition = pending; final controller = WizardGenerationController( service: service, - applyResult: (_) {}, + applyResult: _RecordingApplier().apply, ); addTearDown(controller.dispose); @@ -224,7 +292,7 @@ void main() { final service = _FakeWizardGenerationService(_plan(request)); final controller = WizardGenerationController( service: service, - applyResult: (_) {}, + applyResult: _RecordingApplier().apply, ); addTearDown(controller.dispose); @@ -407,3 +475,28 @@ Slide _generatedSlide(String key) => Slide.parse({ }, ], }); + +/// Test double for [GeneratedDeckResultApplier] that honours the guard the +/// controller passes, exactly as the real applier does after its async work. +final class _RecordingApplier { + final applied = []; + + Completer? started; + Completer? pending; + Object? cleanupError; + + Future apply( + DeckGenerationResult result, { + required GeneratedDeckApplicationGuard isValid, + }) async { + started?.complete(); + if (pending case final pending?) await pending.future; + if (!isValid()) return const GeneratedDeckApplication(published: false); + applied.add(result); + + return GeneratedDeckApplication( + published: true, + cleanupError: cleanupError, + ); + } +} diff --git a/packages/playground/test/features/editor/domain/stores/deck_document_store_test.dart b/packages/playground/test/features/editor/domain/stores/deck_document_store_test.dart index 72d66a7f..de2fdf36 100644 --- a/packages/playground/test/features/editor/domain/stores/deck_document_store_test.dart +++ b/packages/playground/test/features/editor/domain/stores/deck_document_store_test.dart @@ -31,4 +31,20 @@ void main() { expect(notifications, 0); }); + + test('counts only accepted replacements in its revision', () { + final store = DeckDocumentStore(markdown: '# Initial'); + addTearDown(store.dispose); + + expect(store.revision, 0); + + store.replaceMarkdown('# Updated'); + expect(store.revision, 1); + + store.replaceMarkdown('# Updated'); + expect(store.revision, 1); + + store.replaceMarkdown('# Initial'); + expect(store.revision, 2); + }); } diff --git a/packages/playground/test/features/editor/domain/stores/deck_file_session_test.dart b/packages/playground/test/features/editor/domain/stores/deck_file_session_test.dart index 3360e8a2..2992f554 100644 --- a/packages/playground/test/features/editor/domain/stores/deck_file_session_test.dart +++ b/packages/playground/test/features/editor/domain/stores/deck_file_session_test.dart @@ -473,6 +473,23 @@ void main() { expect(repository.accessStarts, [picked]); }); + test('changes the binding revision for identical content', () async { + const picked = DeckFileReference(path: '/elsewhere/same.md'); + final repository = FakeDeckFileRepository() + ..files[picked.path] = kStarterDeckMarkdown + ..pickResult = picked; + final scope = newSession(repository); + final documentRevision = scope.document.revision; + final bindingRevision = scope.session.bindingRevision; + + await scope.session.openDeck(); + + expect(scope.session.boundPath, picked.path); + // The two decks hold identical Markdown, so only the binding moved. + expect(scope.document.revision, documentRevision); + expect(scope.session.bindingRevision, isNot(bindingRevision)); + }); + test('releases the previous bookmark after replacement', () async { const previous = DeckFileReference( path: '/outside/previous.md', diff --git a/packages/playground/test/features/editor/presentation/widgets/editor_header_test.dart b/packages/playground/test/features/editor/presentation/widgets/editor_header_test.dart new file mode 100644 index 00000000..eac79b07 --- /dev/null +++ b/packages/playground/test/features/editor/presentation/widgets/editor_header_test.dart @@ -0,0 +1,122 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:google_fonts/google_fonts.dart'; +import 'package:hero_ui/hero_ui.dart'; +import 'package:playground/core/data/data_sources/memory_deck_loader.dart'; +import 'package:playground/core/domain/stores/deck_customization_store.dart'; +import 'package:playground/features/ai/quick_agent/domain/commands/generate_deck_command.dart'; +import 'package:playground/features/editor/domain/files/deck_file.dart'; +import 'package:playground/features/editor/domain/stores/deck_document_store.dart'; +import 'package:playground/features/editor/domain/stores/deck_file_session.dart'; +import 'package:playground/features/editor/presentation/widgets/editor_header.dart'; +import 'package:provider/provider.dart'; +import 'package:superdeck/superdeck.dart'; + +import '../../../../helpers/fake_deck_file_repository.dart'; + +const _discardNotice = + 'The generated deck was discarded because the document changed ' + 'while it was being created.'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + setUpAll(() => GoogleFonts.config.allowRuntimeFetching = false); + + for (final textScale in [1.0, 1.5]) { + testWidgets( + 'wraps and dismisses a notice in a 600px pane at $textScale text scale', + (tester) async { + final document = DeckDocumentStore(markdown: ''); + final controller = DeckController( + deckLoader: MemoryDeckLoader(), + options: DeckOptions(), + ); + final customization = DeckCustomizationStore(controller); + final session = DeckFileSession( + initialSnapshot: const DeckFileSnapshot( + reference: DeckFileReference(path: '/decks/a.md'), + markdown: '', + ), + repository: FakeDeckFileRepository(), + documentStore: document, + ); + final command = _NoticeCommand( + documentStore: document, + customizationStore: customization, + ); + addTearDown(() { + command.dispose(); + session.dispose(); + customization.dispose(); + controller.dispose(); + document.dispose(); + }); + + await tester.pumpWidget( + MaterialApp( + home: HeroTheme( + data: HeroThemeData.light(), + child: MultiProvider( + providers: [ + ChangeNotifierProvider.value(value: session), + ListenableProvider.value(value: command), + ], + child: MediaQuery( + data: MediaQueryData( + textScaler: TextScaler.linear(textScale), + ), + child: const Scaffold( + body: Center( + child: SizedBox(width: 600, child: EditorHeader()), + ), + ), + ), + ), + ), + ), + ); + + expect(tester.takeException(), isNull); + expect(find.text(_discardNotice), findsOneWidget); + expect( + find.bySemanticsLabel(RegExp('Dismiss generation notice')), + findsOneWidget, + ); + final dismiss = find.byType(HeroIconButton); + final headerRect = tester.getRect(find.byType(EditorHeader)); + final dismissRect = tester.getRect(dismiss); + expect(headerRect.contains(dismissRect.center), isTrue); + expect(dismissRect.right, lessThanOrEqualTo(headerRect.right)); + + await tester.tap(dismiss); + await tester.pumpAndSettle(); + + expect(find.text(_discardNotice), findsNothing); + expect(command.completionNotice, isNull); + expect(tester.takeException(), isNull); + + await tester.pumpWidget(const SizedBox()); + await tester.pump(const Duration(seconds: 2)); + }, + ); + } +} + +/// Supplies the notice while the widget test exercises layout and dismissal. +class _NoticeCommand extends GenerateDeckCommand { + String? _notice = _discardNotice; + + _NoticeCommand({ + required super.documentStore, + required super.customizationStore, + }); + + @override + String? get completionNotice => _notice; + + @override + void dismissNotice() { + _notice = null; + notifyListeners(); + } +}