Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 53 additions & 1 deletion src/components/generator/generator.component.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ describe('GeneratorComponent shared recipe behaviour', () => {
useValue: {
saveRecipe: persistenceSaveRecipe,
saveRecipeDetailed: persistenceSaveRecipe,
// KAN-255 post-image reconcile; null = row unreadable, the
// branch that leaves the optimistic local write standing.
refreshRecipeFromApi: vi.fn().mockResolvedValue(null),
publishStateSync: () => 'synced',
},
},
Expand All @@ -62,7 +65,7 @@ describe('GeneratorComponent shared recipe behaviour', () => {
],
});
const component = runInInjectionContext(injector, () => new GeneratorComponent());
return { component, persistenceSaveRecipe, authUser };
return { component, persistenceSaveRecipe, authUser, recipeState, injector };
};

const draftRecipe = () =>
Expand Down Expand Up @@ -218,4 +221,53 @@ describe('GeneratorComponent shared recipe behaviour', () => {
expect(component.formatAmount(2)).toBe('2');
expect(component.formatAmount([1, 2])).toBe('1 - 2');
});

// KAN-256: `clearRecipe()` fired inside onGenerate() — submit-time, not
// entry-time. The recipe lives on RecipeStateService (a root singleton) so
// it outlived the component, and navigating back to the generator re-showed
// the previous result under an empty prompt box. Route entry recreates the
// component, so constructing a second one IS the repro.
describe('route entry reset (KAN-256)', () => {
it('shows an empty form when the generator is entered again', () => {
const { component, recipeState, injector } = createComponent({ isGuest: false });
recipeState.viewRecipe(draftRecipe());
expect(component.recipe()).not.toBeNull();

const reEntered = runInInjectionContext(injector, () => new GeneratorComponent());

expect(reEntered.recipe()).toBeNull();
expect(reEntered.prompt()).toBe('');
expect(reEntered.error()).toBeNull();
expect(reEntered.isSaved()).toBe(false);
});

it('does not cancel an in-flight image generation for the previous recipe', async () => {
// The spinner and the KAN-255 reconcile are tracked by recipe id on the
// service, so leaving the generator must not discard them — recipe-detail
// still has to show the spinner for that recipe.
const { recipeState, injector } = createComponent({ isGuest: false });
let settle: (url: string) => void = () => {};
recipeState.trackImageGeneration(
'gen-1',
new Promise<string>((resolve) => {
settle = resolve;
})
);
recipeState.viewRecipe(draftRecipe());
expect(recipeState.isImageGenerating()).toBe(true);

runInInjectionContext(injector, () => new GeneratorComponent());

// Cleared here (nothing is being viewed)...
expect(recipeState.currentRecipe()).toBeNull();
// ...but still tracked, so the recipe's own page still spins.
recipeState.viewRecipe(draftRecipe());
expect(recipeState.isImageGenerating()).toBe(true);

settle('/api/recipes/gen-1/image');
await Promise.resolve();
await Promise.resolve();
expect(recipeState.isImageGenerating()).toBe(false);
});
});
});
47 changes: 27 additions & 20 deletions src/components/generator/generator.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,30 @@ export class GeneratorComponent extends RecipeViewBase {
isRecipeLoading = signal(false);
error = signal<string | null>(null);

/**
* KAN-256 — the generator resets on ROUTE ENTRY, not on submit.
*
* `clearRecipe()` used to fire only inside `onGenerate()`, which is
* submit-time. The generated recipe lives on `RecipeStateService`, a
* root-scoped singleton that outlives this component, so navigating away and
* back re-rendered the *previous* recipe under an empty prompt box — the
* "why is my old recipe still here" report.
*
* The constructor is the right hook because route entry is exactly when
* Angular creates this component: navigating to `/` from another route
* destroys and recreates it, while staying on `/` reuses the instance and so
* does not clear a result the user is still reading.
*
* This deliberately does NOT cancel an in-flight image generation. That is
* tracked on the service by recipe id, so the spinner keeps running on
* recipe-detail and the KAN-255 metadata reconcile still lands — the
* generator just is not the surface showing it any more.
*/
constructor() {
super();
this.recipeState.clearRecipe();
}

/**
* A guest activating the publish toggle gets the sign-in modal here, where
* recipe-detail stays silent (its template renders a dedicated "Sign in to
Expand Down Expand Up @@ -44,7 +68,9 @@ export class GeneratorComponent extends RecipeViewBase {
this.recipe.set(generatedRecipe);
this.isSaved.set(true);
await this.persistenceService.saveRecipe(generatedRecipe);
this.triggerImageGeneration(generatedRecipe);
// Fire-and-forget: the image takes far longer than the recipe text, and
// the user must be able to read (and leave) the recipe while it renders.
void this.runImageGeneration(generatedRecipe.id, { regenerate: false });
} catch (err: unknown) {
const message =
err instanceof Error ? err.message : 'Failed to generate recipe. Please try again.';
Expand All @@ -54,25 +80,6 @@ export class GeneratorComponent extends RecipeViewBase {
}
}

async triggerImageGeneration(recipe: Recipe) {
const targetId = recipe.id;
const imagePromise = this.geminiService.generateImage(targetId);
this.recipeState.trackImageGeneration(targetId, imagePromise);
try {
const imageUrl = await imagePromise;
if (this.recipe()?.id === targetId) {
this.generatedImageUrl.set(imageUrl);
this.recipe.update((r) => (r ? { ...r, ai_image_url: imageUrl } : null));
}
this.authService.updateRecipeField(targetId, 'ai_image_url', imageUrl);
} catch (err) {
console.error('Image generation failed', err);
if (this.recipe()?.id === targetId) {
this.toastService.show("Couldn't generate the image. You can retry from the recipe page.");
}
}
}

async onSaveRecipe() {
const currentRecipe = this.recipe();
if (!currentRecipe) return;
Expand Down
166 changes: 164 additions & 2 deletions src/components/shared/recipe-view.base.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { GeminiService } from '../../services/gemini.service';
import { RecipeStateService } from '../../services/recipe-state.service';
import { ToastService } from '../../services/toast.service';
import { ModalService } from '../../services/modal.service';
import { recipeFromRow, type RecipeRow } from '../../utils/recipe-row';

// KAN-126 (#3209): the shared seam between GeneratorComponent and
// RecipeDetailComponent. The component test files cover the behaviour through
Expand All @@ -33,6 +34,8 @@ describe('RecipeViewBase', () => {
isGuest?: boolean;
publishStateSync?: string;
generateImage?: (id: string, force: boolean) => Promise<string>;
refreshRecipeFromApi?: (id: string) => Promise<unknown>;
savedRecipes?: unknown[];
} = {}
) => {
const recipeState = runInInjectionContext(
Expand All @@ -44,7 +47,13 @@ describe('RecipeViewBase', () => {
// both — it is the SaveOutcome the detailed caller reads, and truthy for the
// boolean one.
const persistenceSaveRecipe = vi.fn().mockResolvedValue({ ok: true });
const authUser = { isGuest: opts.isGuest ?? false, savedRecipes: [] as unknown[] };
// KAN-255: the post-image reconcile. Default null = "the row could not be
// read", the branch that must leave the optimistic local write standing.
const refreshRecipeFromApi = vi.fn(opts.refreshRecipeFromApi ?? (async () => null));
const authUser = {
isGuest: opts.isGuest ?? false,
savedRecipes: (opts.savedRecipes ?? []) as unknown[],
};

const injector = Injector.create({
providers: [
Expand All @@ -61,6 +70,7 @@ describe('RecipeViewBase', () => {
useValue: {
saveRecipe: persistenceSaveRecipe,
saveRecipeDetailed: persistenceSaveRecipe,
refreshRecipeFromApi,
publishStateSync: () => opts.publishStateSync ?? 'synced',
},
},
Expand All @@ -82,7 +92,7 @@ describe('RecipeViewBase', () => {

const host = runInInjectionContext(injector, () => new Host());
const authService = injector.get(AuthService);
return { host, persistenceSaveRecipe, recipeState, authService };
return { host, persistenceSaveRecipe, refreshRecipeFromApi, recipeState, authService };
};

it('calls the onPublishDenied hook instead of saving when the user cannot publish', async () => {
Expand Down Expand Up @@ -309,4 +319,156 @@ describe('RecipeViewBase', () => {
expect(payload.ai_image_url).not.toContain('_t=');
});
});

// KAN-255: the image pipeline finishes SERVER-side. The worker writes
// `ai_image_gcs`, `ai_metadata.image_generation`, and flips
// `ai_metadata.image_request.status` / `image_enqueue.status` from `pending`
// to `complete`. The client wrote back only `ai_image_url`, so the copy it
// held — and exported as JSON — still read `pending` with no GCS URI.
describe('server image metadata reconcile (KAN-255)', () => {
const CANONICAL = '/api/recipes/r1/image';
// The row the worker leaves behind, as GET /api/recipes/:id returns it.
const SERVER_ROW = {
id: 'r1',
data: {
id: 'r1',
name: 'Vegan Cornbread',
origin: 'generated',
ai_image_url: CANONICAL,
ai_image_gcs: 'gs://tasteslikegood-recipe-images/r1/claim-abc.png',
ai_metadata: {
image_generation: { success: true, user_display_name: 'Background Worker' },
image_enqueue: { status: 'complete' },
image_request: { id: 'req-1', status: 'complete', force_regenerate: false },
},
},
is_canonical: false,
is_public: false,
slug: null,
source_slug: null,
origin: 'generated',
};

const pendingRecipe = () =>
({
id: 'r1',
name: 'Vegan Cornbread',
origin: 'generated',
ai_metadata: {
image_enqueue: { status: 'pending' },
image_request: { id: 'req-1', status: 'pending', force_regenerate: false },
},
}) as never;

it('re-reads the row and adopts the worker-written fields', async () => {
const { host, refreshRecipeFromApi } = createHost({
generateImage: vi.fn().mockResolvedValue(CANONICAL),
refreshRecipeFromApi: async () => recipeFromRow(SERVER_ROW as unknown as RecipeRow),
});
host.recipe.set(pendingRecipe());

await host.regenerateImage();

expect(refreshRecipeFromApi).toHaveBeenCalledWith('r1');
const adopted = host.recipe() as unknown as typeof SERVER_ROW.data;
// The exact fields that read pending/null before the fix.
expect(adopted.ai_image_gcs).toBe(SERVER_ROW.data.ai_image_gcs);
expect(adopted.ai_metadata.image_request.status).toBe('complete');
expect(adopted.ai_metadata.image_enqueue.status).toBe('complete');
expect(adopted.ai_metadata.image_generation).toBeDefined();
});

it('exports JSON that matches the API row after the reconcile', async () => {
const { host } = createHost({
generateImage: vi.fn().mockResolvedValue(CANONICAL),
refreshRecipeFromApi: async () => recipeFromRow(SERVER_ROW as unknown as RecipeRow),
});
host.recipe.set(pendingRecipe());

await host.regenerateImage();

// exportRecipe stringifies the viewed recipe verbatim; comparing the
// serialized form is the same comparison the AC's repro makes by hand.
expect(JSON.parse(JSON.stringify(host.recipe()))).toEqual(SERVER_ROW.data);
});

// The reconcile GET reads a row that predates anything the user changed
// during the 30-60s image window. Adopting it wholesale reverted that edit
// on screen and in localStorage; only the pipeline fields may be adopted.
it('keeps a notes edit made while the image was still generating', async () => {
const { host } = createHost({
generateImage: vi.fn().mockResolvedValue(CANONICAL),
refreshRecipeFromApi: async () => recipeFromRow(SERVER_ROW as unknown as RecipeRow),
});
host.recipe.set({
...(pendingRecipe() as unknown as Record<string, unknown>),
personalNotes: 'typed during generation',
} as never);

await host.regenerateImage();

const adopted = host.recipe() as unknown as Record<string, unknown>;
expect(adopted['personalNotes']).toBe('typed during generation');
// ...while the worker-written fields still land.
expect(adopted['ai_image_gcs']).toBe(SERVER_ROW.data.ai_image_gcs);
});

it('leaves the optimistic local write standing when the row cannot be read', async () => {
const { host, authService } = createHost({
generateImage: vi.fn().mockResolvedValue(CANONICAL),
refreshRecipeFromApi: async () => null,
});
host.recipe.set(pendingRecipe());

await host.regenerateImage();

expect(authService.updateRecipeField).toHaveBeenCalledWith('r1', 'ai_image_url', CANONICAL);
expect((host.recipe() as { ai_image_url?: string }).ai_image_url).toBe(CANONICAL);
});

it('does not overwrite the viewed recipe when the user has navigated to another one', async () => {
const { host } = createHost({
generateImage: vi.fn().mockResolvedValue(CANONICAL),
refreshRecipeFromApi: async () => recipeFromRow(SERVER_ROW as unknown as RecipeRow),
});
host.recipe.set(pendingRecipe());

const inFlight = host.regenerateImage();
// Nav to a different recipe while the generation is still detached.
host.recipe.set({ id: 'r2', name: 'Chili' } as never);
await inFlight;

expect(host.recipe()?.id).toBe('r2');
});

it('reconciles even after the recipe is no longer the one being viewed', async () => {
// The nav-away repro: the component is gone, the promise is not. The
// local write and the API re-read must BOTH still happen — that is what
// makes the cookbook row correct on return.
const { host, refreshRecipeFromApi, authService } = createHost({
generateImage: vi.fn().mockResolvedValue(CANONICAL),
refreshRecipeFromApi: async () => recipeFromRow(SERVER_ROW as unknown as RecipeRow),
});
host.recipe.set(pendingRecipe());

const inFlight = host.regenerateImage();
host.recipe.set(null);
await inFlight;

expect(authService.updateRecipeField).toHaveBeenCalledWith('r1', 'ai_image_url', CANONICAL);
expect(refreshRecipeFromApi).toHaveBeenCalledWith('r1');
});

it('does not reconcile when generation fails', async () => {
const { host, refreshRecipeFromApi } = createHost({
generateImage: vi.fn().mockRejectedValue(new Error('timed out')),
});
host.recipe.set(pendingRecipe());

await host.regenerateImage();

expect(refreshRecipeFromApi).not.toHaveBeenCalled();
expect(toastShow).toHaveBeenCalledWith("Couldn't regenerate the image. Please try again.");
});
});
});
Loading
Loading