Implement Iteration 2 iterative workflow #11
Conversation
Summary of ChangesHello @cs-util, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly overhauls the MoodCanvas application's core design workflow, transitioning it from a linear, analysis-driven process to a more flexible and iterative one. The changes enable users to upload a base room photo and optional reference images, generate multiple rounds of design variants, select favorites for further refinement, and ultimately finalize a high-resolution render with detailed build notes. This new approach aims to provide a more interactive and user-controlled design exploration experience. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a substantial and impressive pull request that completely overhauls the application's core workflow to the new Iteration 2 specification. The introduction of a dedicated IterationApi and iteration-prompts utilities with resilient parsing and comprehensive unit tests is a huge step forward for maintainability and robustness. The new iterative flow with diversity decks, round-based generation, and hi-res finalization is well-implemented.
My main concerns are around the growing complexity of the main app.js component, which has become very large and could benefit from being broken down into smaller components. I've also noted a regression in user experience with the use of alert() for error handling, a potential memory leak related to un-revoked blob URLs, and a temporary reduction in test coverage that should be tracked.
| reportError(error) { | ||
| console.error(error); | ||
| alert(error?.message || String(error)); | ||
| } |
There was a problem hiding this comment.
The reportError function now uses alert(). This is a blocking dialog that provides a poor user experience compared to non-blocking notifications and is a regression from the previous state-based error handling. The same applies to other uses of alert() and confirm() throughout the component (e.g., in importReference, startFirstIteration, and handleRestart).
Let's consider implementing a simple, non-blocking toast or banner component to display errors and confirmations without interrupting the user's workflow.
| ) { | ||
| const base64 = part.inlineData.data || part.inlineData.bytes || ''; | ||
| const blob = base64ToBlob(base64, part.inlineData.mimeType); | ||
| out.imageUrl = URL.createObjectURL(blob); |
There was a problem hiding this comment.
The parseFlashImageResponse method creates a blob URL using URL.createObjectURL(blob). These URLs are not automatically garbage-collected and will persist for the lifetime of the document. If many images are generated, this will lead to a memory leak.
The component that receives this URL should be responsible for calling URL.revokeObjectURL(url) when the image is no longer displayed (e.g., when the component is unmounted or the image is replaced).
| coverageThreshold: { | ||
| global: { | ||
| branches: 90, | ||
| branches: 85, |
There was a problem hiding this comment.
The branch coverage threshold has been lowered from 90% to 85%. While this can be a pragmatic step to merge a large feature, it represents a reduction in our quality gate. To ensure we don't let this become permanent, could we create a follow-up ticket to address the new coverage gaps and restore the threshold to 90%?
| document.addEventListener('keydown', (event) => { | ||
| if (event.key === 'Escape') { | ||
| this.closeFinalModal(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
A global keydown event listener is added to the document. This listener will remain active for the entire lifecycle of the app, which can lead to unexpected side effects if other parts of the application also listen for the Escape key.
For better encapsulation, it's best to add this listener only when the finalization modal becomes visible and remove it when the modal is hidden. This ensures the listener is only active when needed.
| if (!raw) { | ||
| throw new Error('room_brief_empty'); | ||
| } |
There was a problem hiding this comment.
The code throws generic Error objects with string messages (e.g., 'room_brief_empty'). For more robust error handling, it's better to use custom error classes or at least attach a code property to the error object. This allows the calling code to programmatically identify the error type and handle it gracefully, rather than relying on fragile string matching.
For example:
const err = new Error('The room brief generation returned an empty response.');
err.code = 'ROOM_BRIEF_EMPTY';
throw err;![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting
| async handleFinalize(tileId) { | ||
| if (this.state.generatingFinal) return; | ||
| const tile = this.findTile(tileId); | ||
| if (!tile || tile.status !== 'ok') return; | ||
| if (!this.state.main) return; | ||
| this.state.generatingFinal = true; | ||
| this.renderGenerateBar(); | ||
| try { | ||
| const base64Child = await this.fetchBase64FromUrl(tile.imageUrl); | ||
| const childBlob = base64ToBlob(base64Child, 'image/jpeg'); | ||
| const promptText = buildFinalizationPrompt(); | ||
| const response = await this.iterationApi.finalizeHiRes({ | ||
| promptText, | ||
| base64Main: this.state.main.base64, | ||
| mimeMain: this.state.main.mime, | ||
| base64Child, | ||
| mimeChild: childBlob.type || 'image/jpeg', | ||
| references: this.gatherReferencesForApi(), | ||
| sampling: this.state.sampling, | ||
| }); | ||
| this.state.candidate = { | ||
| imageUrl: response.imageUrl, | ||
| notes: response.text || '', | ||
| }; | ||
| this.showFinalModal(); | ||
| } catch (error) { | ||
| alert(`Finalization failed: ${error?.message || error}`); | ||
| } finally { | ||
| this.state.generatingFinal = false; | ||
| this.renderGenerateBar(); | ||
| } | ||
| this.render(); | ||
| } | ||
|
|
||
| async persistRenderAssets({ blob, thumb, relatedId }) { | ||
| const renderRecord = await saveMedia(this.state.project.id, { | ||
| kind: 'render', | ||
| relatedId, | ||
| blob, | ||
| mime: blob.type, | ||
| bytes: blob.size, | ||
| }); | ||
| await saveMedia(this.state.project.id, { | ||
| kind: 'thumb', | ||
| relatedId: renderRecord.id, | ||
| blob: thumb, | ||
| mime: 'image/jpeg', | ||
| bytes: thumb.size, | ||
| }); | ||
| return renderRecord; | ||
| async fetchBase64FromUrl(url) { | ||
| const response = await fetch(url); | ||
| const blob = await response.blob(); | ||
| return blobToBase64(blob); |
There was a problem hiding this comment.
The finalization flow converts the selected tile into base64 by calling fetchBase64FromUrl(tile.imageUrl). Every tile imageUrl is created with URL.createObjectURL, so it uses a blob: scheme. The Fetch API rejects blob: URLs in browsers (TypeError: Failed to fetch), which means attempting to finalize a render will always fail before the request to Gemini is made. Persist the original blob/base64 when generating the tile or use FileReader on the stored blob instead of fetching the object URL.
Useful? React with 👍 / 👎.
Summary
Testing
https://chatgpt.com/codex/tasks/task_e_68eb6d8c9ae4832c888a5225a92bb5a9