Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
e72d24d
Exclude archived candidates from the rendered video
christophervoelpel Sep 21, 2026
b1972ff
Guard setup inputs on the real project load error and hydration state
christophervoelpel Sep 21, 2026
2dec9be
Offer Setup recovery when full inputs cannot be hydrated
christophervoelpel Sep 21, 2026
d301198
Prevent selection of archived candidates
christophervoelpel Sep 21, 2026
2078f19
Merge branch 'codex/address-pr199-20260921' into codex/address-pr202-…
christophervoelpel Sep 21, 2026
55eba39
Add Apache 2.0 license header to archived-candidate-render.spec.ts
christophervoelpel Sep 22, 2026
e58acf1
Merge remote-tracking branch 'origin/fix/archived-candidate-never-ren…
christophervoelpel Sep 22, 2026
ed7a0eb
Merge remote-tracking branch 'origin/main' into fix/archived-candidat…
christophervoelpel Sep 23, 2026
4397a7b
Align Storyboard, Homepage, and candidate generation with archived-ca…
christophervoelpel Sep 23, 2026
22aa588
Merge branch 'fix/archived-candidate-never-renders' into fix/setup-lo…
christophervoelpel Sep 23, 2026
39c4256
Align candidate move selection/ingredients and suppress transitions a…
christophervoelpel Sep 23, 2026
3c5cd20
Merge branch 'fix/archived-candidate-never-renders' into fix/setup-lo…
christophervoelpel Sep 23, 2026
af19aed
Always select the moved candidate on the destination scene
christophervoelpel Sep 23, 2026
d66b751
Merge branch 'fix/archived-candidate-never-renders' into fix/setup-lo…
christophervoelpel Sep 23, 2026
47230fc
Include candidate thumbnail in test_spa_delivery compression fixture
christophervoelpel Sep 23, 2026
7a56e3a
Merge branch 'fix/archived-candidate-never-renders' into fix/setup-lo…
christophervoelpel Sep 23, 2026
51d3b95
Address review feedback on transition guards, cache invalidation, and…
christophervoelpel Sep 23, 2026
cab7e2f
Merge branch 'fix/archived-candidate-never-renders' into fix/setup-lo…
christophervoelpel Sep 23, 2026
4e2dcff
Merge remote-tracking branch 'origin/main' into perf/deploy-speedup
christophervoelpel Sep 23, 2026
f1ea5f3
fix(ui): keep new projects ready after editor route or failed load
christophervoelpel Sep 24, 2026
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
156 changes: 156 additions & 0 deletions ui/src/app/services/config/config-mediated.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1075,6 +1075,162 @@ describe('ConfigService (mediated data plane)', () => {
errorSpy.mockRestore();
});

it('leaves projectConfig.error() undefined after load failure, setting projectLoadError instead', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
httpClientMock.get.mockImplementation((url: string) => {
if (url === '/api/config') return of({});
return throwError(() => new HttpErrorResponse({status: 500}));
});

service.loadProjectConfig('proj-err', 'full');
await vi.waitFor(() => {
expect(service.projectLoadError()).toBe(true);
});

// rxResource catches the error and degrades to DEFAULT_PROJECT_CONFIG,
// so projectConfig.error() is permanently undefined.
expect(service.projectConfig.error()).toBeUndefined();
expect(service.setupInputsError()).toBe(true);
expect(service.setupInputsLoaded()).toBe(false);
errorSpy.mockRestore();
});

it('evaluates setupInputsLoaded to false when projectLoadError is set even if id matches', () => {
service.projectConfig.value.set({
...service.projectConfig.value(),
id: 'proj-matched',
});
service.loadProjectConfig('proj-matched', 'full');
expect(service.setupInputsLoaded()).toBe(true);

// When a load error is recorded, setupInputsLoaded must be false even though
// value().id === projectId() is true.
(service as any).projectLoadErrorValue.set(new Error('load failed'));
expect(service.projectLoadError()).toBe(true);
expect(service.setupInputsLoaded()).toBe(false);
});

it('evaluates setupInputsLoaded to false on 404 with in-flight autosave to prevent destructive full PATCH', async () => {
const editorProject = {
...service.projectConfig.value(),
id: 'proj-hydrate',
inputConfig: undefined,
};
httpClientMock.get.mockImplementation((url: string) => {
if (url === '/api/config') return of({});
if (url.includes('/api/projects/proj-hydrate')) {
if (url.endsWith('?view=editor')) {
return of(editorProject);
}
return throwError(() => new HttpErrorResponse({status: 404}));
}
return of({});
});

service.loadProjectConfig('proj-hydrate', 'editor');
await vi.waitFor(() => {
expect(service.projectConfig.value().id).toBe('proj-hydrate');
});
markPersisted('proj-hydrate');

const inFlightPatch = new Subject<any>();
httpClientMock.patch.mockReturnValue(inFlightPatch);
service.updateProjectConfig({name: 'in-flight'});
service.saveNow();

service.loadProjectConfig('proj-hydrate', 'full');
await vi.waitFor(() => {
expect(httpClientMock.get).toHaveBeenCalledWith(
'/api/projects/proj-hydrate',
);
});
await Promise.resolve();
await Promise.resolve();

// On 404 with localProjectAtLoad, the loader preserves local latestSource so the user
// is not disrupted, but setupInputsLoaded MUST be false because full server inputs were not hydrated.
expect(service.setupInputsLoaded()).toBe(false);
expect(service.projectConfig.value().id).toBe('proj-hydrate');
expect(service.projectConfig.value().inputConfig).toBeUndefined();
expect(service.setupInputsError()).toBe(true);
expect(service.projectLoadError()).toBe(false);
});

it('navigates home and resets project on 404 without local copy', async () => {
const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
httpClientMock.get.mockImplementation((url: string) => {
if (url === '/api/config') return of({});
return throwError(() => new HttpErrorResponse({status: 404}));
});

service.loadProjectConfig('non-existent', 'full');
await vi.waitFor(() => {
expect(httpClientMock.get).toHaveBeenCalledWith(
'/api/projects/non-existent',
);
});
await Promise.resolve();
await Promise.resolve();

expect(service.setupInputsLoaded()).toBe(false);
expect(service.projectConfig.value().id).toBe('');
expect(TestBed.inject(Router).navigate).toHaveBeenCalledWith(['/']);
errorSpy.mockRestore();
});

it('yields setupInputsLoaded === true on normal successful load', async () => {
const fullProject = {
...service.projectConfig.value(),
id: 'proj-ok',
inputConfig: {products: [], composition: 'ok'},
};
httpClientMock.get.mockImplementation((url: string) => {
if (url === '/api/config') return of({});
if (url === '/api/projects/proj-ok') return of(fullProject);
return of({});
});

service.loadProjectConfig('proj-ok', 'full');
await vi.waitFor(() => {
expect(service.projectConfig.value().id).toBe('proj-ok');
});

expect(service.projectLoadError()).toBe(false);
expect(service.setupInputsLoaded()).toBe(true);
});

it('keeps a locally created project ready after leaving an editor route or failed load', async () => {
httpClientMock.get.mockImplementation((url: string) => {
if (url === '/api/config') return of({});
if (url === '/api/projects/project-a?view=editor') {
return of({...service.projectConfig.value(), id: 'project-a'});
}
return of({});
});
service.loadProjectConfig('project-a', 'editor');
await vi.waitFor(() => {
expect(service.projectConfig.value().id).toBe('project-a');
});
(service as any).projectLoadErrorValue.set(new Error('stale failure'));
httpClientMock.get.mockClear();

service.resetProjectConfig();
service.setNewProject('project-b');
expect(service.setupInputsLoaded()).toBe(true);
service.saveNow();

service.loadProjectConfig('project-b', 'full');
await new Promise(resolve => setTimeout(resolve, 0));

expect(httpClientMock.get).not.toHaveBeenCalledWith(
'/api/projects/project-b',
);
expect(service.projectLoadError()).toBe(false);
expect(service.setupInputsError()).toBe(false);
expect(service.setupInputsLoaded()).toBe(true);
expect(service.projectConfig.value().id).toBe('project-b');
});

it('keeps a locally created full project ready after leaving another project', () => {
service.projectConfig.value.set({
...service.projectConfig.value(),
Expand Down
42 changes: 35 additions & 7 deletions ui/src/app/services/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -644,6 +644,7 @@ export class ConfigService {
private projectId = signal<string | null>(null);
private projectView = signal<ProjectConfigView>('full');
private projectLoadErrorValue = signal<unknown>(undefined);
private setupInputsHydrated = signal<boolean>(false);
/**
* Mediated mode only: ids known to exist server-side (loaded via GET or
* already POSTed). First save of a new project goes through
Expand Down Expand Up @@ -946,6 +947,7 @@ export class ConfigService {
loader: async ({params, abortSignal}) => {
if (params.projectId === null) {
this.projectLoadErrorValue.set(undefined);
this.setupInputsHydrated.set(false);
return {...this.DEFAULT_PROJECT_CONFIG()};
}
const isCurrentLoad = () =>
Expand All @@ -954,6 +956,7 @@ export class ConfigService {
this.projectView() === params.view;
if (isCurrentLoad()) {
this.projectLoadErrorValue.set(undefined);
this.setupInputsHydrated.set(false);
}
const localProjectAtLoad = this.projectWithUnsettledSave(
params.projectId,
Expand All @@ -967,6 +970,9 @@ export class ConfigService {
if (isCurrentLoad()) {
this.projectLoadErrorValue.set(undefined);
this.persistedProjectIds.add(params.projectId);
if (params.view === 'full') {
this.setupInputsHydrated.set(true);
}
}
if (localProjectAtLoad) {
const latest =
Expand Down Expand Up @@ -1021,16 +1027,32 @@ export class ConfigService {
(!!this.projectLoadErrorValue() || !!this.projectConfig.error()),
);
readonly setupInputsError = computed(
() => this.projectView() === 'full' && this.projectLoadError(),
);
readonly setupInputsLoaded = computed(
() =>
this.projectView() === 'full' &&
!this.projectConfig.isLoading() &&
!this.projectConfig.error() &&
(this.projectConfig.value().id === this.projectId() ||
(this.projectId() === null && !!this.projectConfig.value().id)),
(this.projectLoadError() ||
(this.projectId() !== null &&
!this.projectConfig.isLoading() &&
this.projectConfig.value().id === this.projectId() &&
!this.setupInputsHydrated())),
);
/**
* Setup may expose its input fields only when the current project's
* inputConfig came from a successful full server load, or the project was
* created locally.
*/
readonly setupInputsLoaded = computed(() => {
const isLocalNewProject =
this.projectId() === null && !!this.projectConfig.value().id;
const isHydratedServerProject =
this.setupInputsHydrated() &&
this.projectConfig.value().id === this.projectId();
return (
this.projectView() === 'full' &&
!this.projectConfig.isLoading() &&
!this.projectLoadError() &&
(isLocalNewProject || isHydratedServerProject)
);
});

private normalizeLoadedProject(data: ProjectConfig): ProjectConfig {
if (data.renderRuns) {
Expand Down Expand Up @@ -1491,7 +1513,10 @@ export class ConfigService {
// would otherwise be silently dropped once the config resets (the
// post-reset emission has id === '' / shouldSave === false).
this.flushPendingSave();
this.projectLoadErrorValue.set(undefined);
this.setupInputsHydrated.set(false);
this.projectId.set(null);
this.projectView.set('full');
this.projectConfig.set({...this.DEFAULT_PROJECT_CONFIG()});
this.shouldSave = false;
}
Expand All @@ -1510,6 +1535,9 @@ export class ConfigService {
// Not persisted yet: the first autosave POSTs /api/projects, where the
// server stamps createdBy from the verified identity. Left undefined here.
this.persistedProjectIds.delete(uuid);
this.projectLoadErrorValue.set(undefined);
this.setupInputsHydrated.set(true);
Comment on lines 1517 to +1539

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When leaving an editor route (/:id/storyboard, /:id/composition, or /:id/output-video where this.projectView() is 'editor') or leaving a failed Setup load via Back to projects ('/'), resetProjectConfig() and setNewProject() do not reset this.projectView to 'full' or clear this.projectLoadErrorValue.

Consequently, when creating a new project after visiting an editor route (loadProjectConfig('project-a', 'editor') -> resetProjectConfig() -> setNewProject('project-b') -> saveNow() -> loadProjectConfig('project-b', 'full')):

  1. this.projectView() remains 'editor', so this.setupInputsLoaded() is false immediately after setNewProject('project-b').
  2. When loadProjectConfig('project-b', 'full') runs on navigation to /:id/setup, the short-circuit guard view === this.projectView() || (view === 'editor' && alreadyFull) at line 1587 evaluates to false because this.projectView() is still 'editor'.
  3. loadProjectConfig then sets this.projectId.set('project-b'), which causes the projectConfig loader to reset this.setupInputsHydrated.set(false) and dispatch a GET /api/projects/project-b that races the in-flight POST /api/projects (and triggers setupInputsError() === true if the GET returns 404 before the POST commits).

Resetting projectLoadErrorValue, projectId, and projectView in resetProjectConfig() and setNewProject() ensures locally created projects remain hydrated and short-circuit loadProjectConfig regardless of the previous route's view mode.

Suggested change
this.projectLoadErrorValue.set(undefined);
this.setupInputsHydrated.set(false);
this.projectId.set(null);
this.projectView.set('full');
this.projectConfig.set({...this.DEFAULT_PROJECT_CONFIG()});
this.shouldSave = false;
}
updateProjectConfig(partial: Partial<ProjectConfig>) {
this.shouldSave = true;
this.projectConfig.update(config => {
return {
...config,
...partial,
};
});
}
setNewProject(uuid: string) {
// Not persisted yet: the first autosave POSTs /api/projects, where the
// server stamps createdBy from the verified identity. Left undefined here.
this.persistedProjectIds.delete(uuid);
this.projectLoadErrorValue.set(undefined);
this.setupInputsHydrated.set(true);
this.projectId.set(null);
this.projectView.set('full');

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, I reproduced it. After an editor route (or a failed load), projectView stayed 'editor' and the stale projectLoadErrorValue stayed set. So after "New project", setupInputsLoaded() was false, and Setup was stuck until a reload.

Fix (config.ts):

  • resetProjectConfig() now also clears projectLoadErrorValue and resets projectView to 'full'.
  • setNewProject() now also clears projectLoadErrorValue and resets projectView to 'full'. Both happen before projectConfig.set(project), so the resource can't reload and overwrite the local project.
  • I deliberately did not null projectId inside setNewProject(). A first attempt did, and it broke the existing does not mark a recreated project persisted from a stale load success test, which relies on that contract. resetProjectConfig() already nulls it on the normal "leave project" path.

Test: keeps a locally created project ready after leaving an editor route or failed load in config-mediated.spec.ts. It covers: editor route, then a stale load error, then resetProjectConfig(), then setNewProject(). It then asserts that setupInputsLoaded() is true and stays true after loadProjectConfig(id, 'full'), with no GET /api/projects/<new>, no setupInputsError, and no projectLoadError.

Mutation check: I reverted only the config.ts hunk and this test fails (1 failed / 90 passed); with the fix restored it passes.

Full gate: compile, lint, typecheck:spec, the full UI test suite, and the Python suite are all green (details in the commit).

this.projectView.set('full');
const project = {
...this.DEFAULT_PROJECT_CONFIG(),
id: uuid,
Expand Down
1 change: 1 addition & 0 deletions ui/src/app/setup/setup.html
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
<button mat-button type="button" (click)="config.reloadProjectConfig()">
Retry
</button>
<a mat-button routerLink="/">Back to projects</a>
</div>
} @else if (setupLoading() || !setupReady()) {
<div class="loading-state">
Expand Down
115 changes: 115 additions & 0 deletions ui/src/app/setup/setup.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,121 @@ describe('Setup full-load failure', () => {
fixture.nativeElement.querySelector('.setup-container'),
).not.toBeNull();
});

it('does not show an error before a project load or while the load is pending', () => {
expect(config.setupInputsError()).toBe(false);
expect(fixture.nativeElement.textContent).not.toContain(
'Could not load this project',
);
config.loadProjectConfig('pending-project', 'full');
expect(config.setupInputsError()).toBe(false);
TestBed.tick();
fixture.detectChanges();
expect(fixture.nativeElement.querySelector('mat-spinner')).not.toBeNull();
expect(config.setupInputsError()).toBe(false);
http
.expectOne('/api/projects/pending-project')
.flush('failed', {status: 500, statusText: 'Server Error'});
});

it('offers recovery without blank inputs or full PATCH after both load and pending save return 404', async () => {
(
config as unknown as {persistedProjectIds: Set<string>}
).persistedProjectIds.add('proj-unsettled');

config.loadProjectConfig('proj-unsettled', 'editor');
TestBed.tick();
const editorReq = http.expectOne(
'/api/projects/proj-unsettled?view=editor',
);
editorReq.flush({
id: 'proj-unsettled',
name: 'Editor Project',
aspectRatio: '16:9',
resolution: '720p',
candidateDurationSeconds: 4,
generateAudio: false,
numberOfCandidates: 1,
model: 'veo-default',
inputConfig: undefined,
storyboard: [],
audioTracks: [],
visualOverlays: [],
});
TestBed.tick();
await fixture.whenStable();

config.updateProjectConfig({name: 'In-Flight'});
config.saveNow();
const inFlightPatch = http.expectOne('/api/projects/proj-unsettled/editor');

config.loadProjectConfig('proj-unsettled', 'full');
expect(config.setupInputsError()).toBe(false);
TestBed.tick();
expect(config.setupInputsError()).toBe(false);
const fullGet = http.expectOne('/api/projects/proj-unsettled');
fullGet.flush('Not found', {status: 404, statusText: 'Not Found'});
TestBed.tick();
await fixture.whenStable();
fixture.detectChanges();

expect(config.setupInputsLoaded()).toBe(false);
expect(config.projectConfig.value().inputConfig).toBeUndefined();
http.expectNone('/api/projects/proj-unsettled');

inFlightPatch.flush('Not found', {status: 404, statusText: 'Not Found'});
TestBed.tick();
await fixture.whenStable();
fixture.detectChanges();

expect(config.projectLoadError()).toBe(false);
expect(config.setupInputsError()).toBe(true);
expect(fixture.nativeElement.querySelector('mat-spinner')).toBeNull();
expect(fixture.nativeElement.textContent).toContain(
'Could not load this project',
);
const home = fixture.nativeElement.querySelector('a[href="/"]');
expect(home?.textContent).toContain('Back to projects');
expect(fixture.nativeElement.querySelector('.setup-container')).toBeNull();
expect(config.projectConfig.value().inputConfig).toBeUndefined();
http.expectNone('/api/projects/proj-unsettled');

const retry = fixture.nativeElement.querySelector(
'.loading-state button',
) as HTMLButtonElement | null;
expect(retry?.textContent).toContain('Retry');
retry?.click();
TestBed.tick();
const retryGet = http.expectOne('/api/projects/proj-unsettled');
retryGet.flush({
id: 'proj-unsettled',
name: 'Server Name',
aspectRatio: '16:9',
resolution: '720p',
candidateDurationSeconds: 4,
generateAudio: false,
numberOfCandidates: 1,
model: 'veo-default',
inputConfig: {products: [], composition: 'Recovered composition'},
storyboard: [],
audioTracks: [],
visualOverlays: [],
});
TestBed.tick();
await fixture.whenStable();
fixture.detectChanges();

expect(config.setupInputsError()).toBe(false);
expect(config.setupInputsLoaded()).toBe(true);
expect(config.projectConfig.value().name).toBe('In-Flight');
expect(config.projectConfig.value().inputConfig).toEqual({
products: [],
composition: 'Recovered composition',
});
expect(
fixture.nativeElement.querySelector('.setup-container'),
).not.toBeNull();
});
Comment on lines +199 to +240

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the double-404 recovery test (projectLoadError() === false with !setupInputsHydrated()), we verify that Back to projects is rendered, but we don't currently exercise clicking Retry (config.reloadProjectConfig()) from this state to confirm that a subsequent 200 OK response hydrates inputConfig, preserves the unsaved local editor edit (name: 'In-Flight'), clears setupInputsError(), and opens .setup-container.

Suggested change
const home = fixture.nativeElement.querySelector('a[href="/"]');
expect(home?.textContent).toContain('Back to projects');
expect(fixture.nativeElement.querySelector('.setup-container')).toBeNull();
expect(config.projectConfig.value().inputConfig).toBeUndefined();
http.expectNone('/api/projects/proj-unsettled');
});
const home = fixture.nativeElement.querySelector('a[href="/"]');
expect(home?.textContent).toContain('Back to projects');
expect(fixture.nativeElement.querySelector('.setup-container')).toBeNull();
expect(config.projectConfig.value().inputConfig).toBeUndefined();
http.expectNone('/api/projects/proj-unsettled');
const retry = fixture.nativeElement.querySelector(
'.loading-state button',
) as HTMLButtonElement | null;
expect(retry?.textContent).toContain('Retry');
retry?.click();
TestBed.tick();
const retryGet = http.expectOne('/api/projects/proj-unsettled');
retryGet.flush({
id: 'proj-unsettled',
name: 'Server Name',
aspectRatio: '16:9',
resolution: '720p',
candidateDurationSeconds: 4,
generateAudio: false,
numberOfCandidates: 1,
model: 'veo-default',
inputConfig: {products: [], composition: 'Recovered composition'},
storyboard: [],
audioTracks: [],
visualOverlays: [],
});
TestBed.tick();
await fixture.whenStable();
fixture.detectChanges();
expect(config.setupInputsError()).toBe(false);
expect(config.setupInputsLoaded()).toBe(true);
expect(config.projectConfig.value().name).toBe('In-Flight');
expect(config.projectConfig.value().inputConfig).toEqual({
products: [],
composition: 'Recovered composition',
});
expect(
fixture.nativeElement.querySelector('.setup-container'),
).not.toBeNull();
});

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, and applied as suggested (unchanged). The double-404 recovery test now also clicks Retry and flushes a 200. It asserts:

  • setupInputsError() is false;
  • setupInputsLoaded() is true;
  • the unsaved local edit name: 'In-Flight' is kept;
  • inputConfig is hydrated to Recovered composition;
  • .setup-container renders.

Mutation check: I turned reloadProjectConfig() into a no-op. This test fails, along with 2 other retry tests; with the method restored they all pass.

});

describe('Setup image upload', () => {
Expand Down
Loading