-
Notifications
You must be signed in to change notification settings - Fork 208
Fix: Prevnet early dpeloynt success screen #2777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Console (appwrite/console)Project ID: Tip Teams feature lets you group users with membership management and role permissions |
WalkthroughgetEffectiveBuildStatus now accepts a Models.Deployment and regional console variables, deriving original status, creation time, and screenshot presence from the deployment; it returns 'finalizing' when a deployment is ready but missing screenshots and otherwise preserves prior timeout logic. Call sites across functions, sites, and deployment components were updated to pass the deployment object and regional variables. The create-site deploying page adds an exported goToFinishScreen(), a nullable skipScreenshotTimeout timer, a derived effectiveStatus from the new helper, a 30s fallback that navigates to the finish screen only when deployment is ready and both screenshots exist, and cleanup of realtime subscriptions and the timeout on unmount. UI rendering treats 'finalizing' alongside 'building' where applicable. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte:
- Around line 28-34: Add a 60s timeout and failure fallback around the
screenshot wait: start a timer when this component mounts that forces the same
navigation that currently runs inside the if (isReady) block after 60 seconds
unless cleared; treat deployment.status values like 'failed' or 'error' as
immediate fallbacks to navigate with an error flag; detect explicit screenshot
generation errors (e.g., null/undefined or an error field on deployment) and
navigate with a fallback screenshot or error state; always clear the timeout
when isReady becomes true or when you navigate due to failure so you don’t
double-navigate.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
Learnt from: ItzNotABug
Repo: appwrite/console PR: 2316
File: src/routes/(console)/project-[region]-[project]/functions/create-function/deploy/+page.svelte:95-103
Timestamp: 2025-09-08T13:21:53.793Z
Learning: In Appwrite's one-click deployment flows, Git tag validation should be lenient with fallbacks (like '1.0.0') rather than strict validation that blocks deployments, since the source code retrieval from the repository is more important than having proper version tags for the deployment to succeed.
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Cursor integration URL format `https://cursor.com/link/prompt` with the `text` query parameter is correct and functional.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes a timing issue where users could prematurely navigate to the deployment success screen before screenshots were fully generated. Due to a backend change that made screenshots asynchronous, the frontend now needs to wait for both light and dark theme screenshots to be available before proceeding.
Key Changes:
- Added explicit checks for
screenshotLightandscreenshotDarkproperties alongside the deployment ready status
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte (1)
91-96: Include 'finalizing' state in loading and cancel conditions.The new 'finalizing' intermediate state (for ready-but-missing-screenshots builds) should be treated like 'building' in the UI. Currently, deployments in the 'finalizing' state won't show loading spinners or allow cancellation.
🔧 Proposed fix to handle 'finalizing' state
- {#if effectiveStatus === 'processing' || effectiveStatus === 'building' || effectiveStatus === 'waiting'} + {#if effectiveStatus === 'processing' || effectiveStatus === 'building' || effectiveStatus === 'waiting' || effectiveStatus === 'finalizing'} <Button text on:click={() => {- {#if ['processing', 'building'].includes(effectiveStatus)} + {#if ['processing', 'building', 'finalizing'].includes(effectiveStatus)} <Typography.Code color="--fgcolor-neutral-secondary"> <Layout.Stack direction="row" alignItems="center" inline>Also applies to: 180-186
src/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.svelte (1)
93-121: Include 'finalizing' state in download and cancel action conditions.The 'finalizing' state should be treated like 'building'. Deployments in this state should allow source downloads and cancellation, but currently these actions won't be available.
🔧 Proposed fix to handle 'finalizing' state
- {#if effectiveStatus === 'ready' || effectiveStatus === 'failed' || effectiveStatus === 'building'} + {#if effectiveStatus === 'ready' || effectiveStatus === 'failed' || effectiveStatus === 'building' || effectiveStatus === 'finalizing'} <SubMenu> <ActionMenu.Root noPadding>- {#if effectiveStatus === 'processing' || effectiveStatus === 'building' || effectiveStatus === 'waiting'} + {#if effectiveStatus === 'processing' || effectiveStatus === 'building' || effectiveStatus === 'waiting' || effectiveStatus === 'finalizing'} <ActionMenu.Item.Button leadingIcon={IconXCircle}Also applies to: 123-134
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.svelte (1)
126-132: Include 'finalizing' state in build duration, cancel, and delete conditionals.The 'finalizing' state should be treated like 'building' across multiple UI conditionals:
- Build duration should show a timer (not a fixed duration)
- Cancel action should be available
- Delete action should be disabled
Currently, deployments in 'finalizing' state will show incorrect UI states.
🔧 Proposed fix to handle 'finalizing' state
{:else if column.id === 'buildDuration'} {#if ['waiting'].includes(effectiveStatus)} - - {:else if ['processing', 'building'].includes(effectiveStatus)} + {:else if ['processing', 'building', 'finalizing'].includes(effectiveStatus)} <span use:timer={{ start: deployment.$createdAt }}></span>- {#if effectiveStatus === 'processing' || effectiveStatus === 'building' || effectiveStatus === 'waiting'} + {#if effectiveStatus === 'processing' || effectiveStatus === 'building' || effectiveStatus === 'waiting' || effectiveStatus === 'finalizing'} <ActionMenu.Item.Button trailingIcon={IconXCircle}- {#if effectiveStatus !== 'building' && effectiveStatus !== 'processing' && effectiveStatus !== 'waiting'} + {#if effectiveStatus !== 'building' && effectiveStatus !== 'processing' && effectiveStatus !== 'waiting' && effectiveStatus !== 'finalizing'} <ActionMenu.Item.Button leadingIcon={IconTrash}Also applies to: 183-195, 196-209
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.svelte (1)
86-88: Add explicit handling for 'finalizing' status in deploymentStatusConverter.The function currently lacks a case for 'finalizing' status, which is returned by
getEffectiveBuildStatus()fromsrc/lib/helpers/buildTimeout.ts. When 'finalizing' is passed todeploymentStatusConverter, it falls through to the default case and returns 'processing', potentially displaying an incorrect status indicator. Add an explicit case to map 'finalizing' to the appropriate Status component value.
🤖 Fix all issues with AI agents
In
@src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte:
- Line 98: The template is checking
['processing','building','finalizing'].includes(data.deployment.status) which
will never match 'finalizing' because it uses the initial data
(data.deployment.status) and the backend returns raw 'ready' while 'finalizing'
is computed by getEffectiveBuildStatus(); change the condition to use the
reactive deployment.status or call getEffectiveBuildStatus() here and check that
result for 'finalizing', or directly test the underlying condition (e.g., the
awaiting-screenshots flag/logic) instead of relying on data.deployment.status.
- Around line 37-44: Multiple interval bugs: ensure the fallback timer is
managed to avoid leaks and duplicate navigations by (1) clearing any existing
skipScreenshotInterval before creating a new timer when isReady becomes true,
(2) clearing skipScreenshotInterval immediately after invoking
goToFinishScreen() in the fallback path, and (3)
clearing/clearing-and-nullifying skipScreenshotInterval in the onMount cleanup
return so it’s removed when the component unmounts; prefer using setTimeout
instead of setInterval for a one-shot 30s fallback and update all references to
skipScreenshotInterval, goToFinishScreen, isReady, isFinished, and the onMount
cleanup accordingly.
🧹 Nitpick comments (3)
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.svelte (1)
115-116: Consider including 'finalizing' in the timer display condition.The timer currently shows for
['processing', 'building']states. Since 'finalizing' is also an in-progress state (waiting for screenshots), you might want to include it here to show a running timer during finalization.♻️ Optional enhancement
- {:else if ['processing', 'building'].includes(effectiveStatus)} + {:else if ['processing', 'building', 'finalizing'].includes(effectiveStatus)} <span use:timer={{ start: deployment.$createdAt }}></span>src/lib/helpers/buildTimeout.ts (1)
35-45: Consider simplifying the screenshot check logic.The current implementation is correct but verbose. This can be condensed:
♻️ Suggested simplification
const isReady = originalStatus === 'ready'; - let hasScreenshot = true; - if (screenshots.length === 0) { - hasScreenshot = false; - } - for (const screenshot of screenshots) { - if (!screenshot) { - hasScreenshot = false; - break; - } - } + const hasScreenshot = screenshots.length > 0 && screenshots.every(Boolean);src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (1)
19-19: Add type annotation for the interval variable.For clarity and IDE support, add a type annotation. Also, if switching to
setTimeoutper the earlier suggestion, update accordingly.♻️ Suggested change
- let skipScreenshotInterval = $state(null); + let skipScreenshotInterval: ReturnType<typeof setTimeout> | null = $state(null);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.svelte
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.sveltesrc/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.sveltesrc/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/lib/helpers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement pure functions in src/lib/helpers/ directory and use camelCase for helper function names
Files:
src/lib/helpers/buildTimeout.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/lib/helpers/buildTimeout.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
📚 Learning: 2025-09-24T10:27:36.797Z
Learnt from: vermakhushboo
Repo: appwrite/console PR: 2364
File: src/routes/(console)/project-[region]-[project]/sites/site-[site]/domains/add-domain/verify-[domain]/+page.svelte:52-89
Timestamp: 2025-09-24T10:27:36.797Z
Learning: In sites domain verification flows, domain creation should be handled in the background using promise chains (.then/.catch) with silent error handling, not awaited synchronously. This prevents domain creation failures from interrupting the main verification flow.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (12)
src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte (1)
47-47: LGTM! Screenshot data correctly passed to build status helper.The addition of screenshot data to
getEffectiveBuildStatusaligns with the PR objective to prevent early navigation to the success screen before screenshots are ready.src/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.svelte (1)
80-80: LGTM! Screenshot data correctly passed to build status helper.The addition of screenshot data aligns with the PR's goal to consider screenshots when determining deployment status.
src/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.svelte (1)
59-59: LGTM! Screenshot data correctly passed to build status helper.The updated call signature properly provides screenshot data for effective status computation.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.svelte (1)
100-100: LGTM! Screenshot data correctly passed to build status helper.The updated call signature aligns with the new API for computing effective build status with screenshot data.
src/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.svelte (1)
13-19: Excellent! Correct handling of screenshot data and 'finalizing' state.This file serves as the reference implementation:
- Screenshot data is properly passed to
getEffectiveBuildStatus- The 'finalizing' state is correctly included in the loading conditions alongside 'processing' and 'building'
This ensures deployments that are ready but waiting for screenshots show appropriate loading indicators.
Also applies to: 23-23
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.svelte (2)
9-9: LGTM! Consistent status handling.The 'finalizing' state is correctly mapped to 'warning' badge type, consistent with 'building' status. This is appropriate since finalization is still an in-progress state.
44-51: LGTM! API update is correct.The
getEffectiveBuildStatuscall is correctly updated to pass the screenshots array and regional console variables as the new third and fourth parameters.src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.svelte (1)
84-89: LGTM! Consistent API usage.The
getEffectiveBuildStatuscall is correctly updated with the screenshots array parameter, consistent with the API changes across the codebase.src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.svelte (1)
45-52: LGTM! Correct API integration.The derived
effectiveStatusnow correctly incorporates screenshot data via the updatedgetEffectiveBuildStatussignature.src/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.svelte (1)
41-48: LGTM! Consistent cross-feature update.The API update is correctly applied to the functions feature as well, maintaining consistency across the codebase.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte (1)
29-36: LGTM! Correct implementation.The
effectiveStatusderivation correctly integrates the new screenshot parameters, ensuring accurate status computation on the deployment detail page.src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (1)
49-58: LGTM!Clean extraction of the navigation logic into a reusable helper function.
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.svelte:66
- The setCopy function doesn't handle the 'finalizing' status. When a deployment is in the 'finalizing' state (ready but waiting for screenshots), it will fall through to the default case and show the emptyCopy message. Consider adding a case for 'finalizing' to provide appropriate feedback, such as "Finalizing deployment...".
function setCopy() {
if (effectiveStatus === 'failed') {
return 'Your deployment has failed.';
} else if (effectiveStatus === 'building') {
//Do not remove empty space before the string it's an invisible character
return '�[37mPreparing for build ... �[0m\n';
} else if (effectiveStatus === 'waiting') {
return '�[37mPreparing for build ... �[0m\n';
} else if (effectiveStatus === 'processing') {
return '�[37mPreparing for build ... �[0m\n';
} else {
return emptyCopy;
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte:
- Around line 57-66: The code uses setInterval for a one-time 30s fallback
(skipScreenshotInterval => setInterval(..., 30000)) which should be a
setTimeout; change the timer creation to use setTimeout and rename the variable
to reflect a timeout (or keep name but treat as timeout), ensure the cleanup
uses clearTimeout(skipScreenshotInterval) instead of clearInterval, and leave
the existing immediate clear when isReady && isFinished to clear the timeout
before calling goToFinishScreen; update all references to skipScreenshotInterval
and clearInterval to use the timeout variant so the fallback fires only once.
🧹 Nitpick comments (1)
src/lib/helpers/buildTimeout.ts (1)
37-51: Simplify screenshot validation logic.The check on lines 39-41 is dead code since callers always pass an array of length 2 (
[deployment.screenshotLight, deployment.screenshotDark]). The for loop correctly validates the screenshots, but the entire validation could be more concise.♻️ Proposed simplification
- const isReady = originalStatus === 'ready'; - let hasScreenshot = true; - if (screenshots.length === 0) { - hasScreenshot = false; - } - for (const screenshot of screenshots) { - if (!screenshot) { - hasScreenshot = false; - break; - } - } - - if (isReady && !hasScreenshot) { + const isReady = originalStatus === 'ready'; + const hasScreenshot = screenshots.length > 0 && screenshots.every(Boolean); + + if (isReady && !hasScreenshot) { return 'finalizing'; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/lib/helpers/buildTimeout.ts
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/lib/helpers/buildTimeout.ts
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/lib/helpers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement pure functions in src/lib/helpers/ directory and use camelCase for helper function names
Files:
src/lib/helpers/buildTimeout.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/lib/helpers/buildTimeout.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Agent
- GitHub Check: e2e
- GitHub Check: build
🔇 Additional comments (3)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (3)
23-30: LGTM!The derived
effectiveStatuscorrectly passes the screenshot array and regional variables togetEffectiveBuildStatus, enabling the 'finalizing' state when screenshots are pending.
77-86: LGTM!The
goToFinishScreenhelper correctly resolves the URL using the resolve utility and navigates with the site ID as a query parameter. The implementation follows SvelteKit conventions.
126-126: LGTM!The footer condition correctly includes
'finalizing'in the status check and useseffectiveStatusto account for the screenshot-gated state. The background deployment message is appropriate for all three states.
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Show resolved
Hide resolved
...roject-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.svelte
Outdated
Show resolved
Hide resolved
.../project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@src/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.svelte:
- Line 10: The type annotation on the destructured props includes an unused
`status: string`; update the destructuring declaration so it only types
`deployment` (e.g., change the annotation on `let { deployment }` from `{
status: string; deployment: Models.Deployment }` to just `{ deployment:
Models.Deployment }` or a plain `Models.Deployment`) to match the actual
destructured prop from `$props()`.
In
@src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte:
- Around line 50-54: The fallback currently uses setInterval to call
goToFinishScreen() every 30s; change the one-time fallback to setTimeout
(replace setInterval with setTimeout when assigning skipScreenshotInterval),
rename or treat skipScreenshotInterval as a timeout id, and in the component
cleanup replace clearInterval(skipScreenshotInterval) with
clearTimeout(skipScreenshotInterval) so the timeout is cleared correctly when
the component unmounts; keep the same delay (30000) and leave the isReady check
and goToFinishScreen() call unchanged.
🧹 Nitpick comments (1)
src/lib/helpers/buildTimeout.ts (1)
38-47: Simplify the screenshot check—thescreenshots.length === 0condition is dead code.The
screenshotsarray is always initialized with exactly 2 elements (line 30), soscreenshots.length === 0will never be true. The logic can be simplified.♻️ Suggested simplification
- let hasScreenshot = true; - if (screenshots.length === 0) { - hasScreenshot = false; - } - for (const screenshot of screenshots) { - if (!screenshot) { - hasScreenshot = false; - break; - } - } + const hasScreenshot = Boolean(deployment.screenshotLight && deployment.screenshotDark);
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
src/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.svelte
🚧 Files skipped from review as they are similar to previous changes (3)
- src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/table.svelte
- src/routes/(console)/project-[region]-[project]/functions/function-[function]/(components)/deploymentCard.svelte
- src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
src/lib/helpers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement pure functions in src/lib/helpers/ directory and use camelCase for helper function names
Files:
src/lib/helpers/buildTimeout.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/lib/helpers/buildTimeout.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/table.sveltesrc/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to e2e/journeys/*.spec.ts : Write Playwright E2E tests in e2e/journeys/*.spec.ts files with 120s timeout and 3x retry configuration
Applied to files:
src/lib/helpers/buildTimeout.ts
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/lib/helpers/*.test.ts : Write Vitest unit tests in src/lib/helpers/*.test.ts files with both client and server test setup support
Applied to files:
src/lib/helpers/buildTimeout.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (15)
src/routes/(console)/project-[region]-[project]/sites/(components)/deploymentActionMenu.svelte (1)
56-59: LGTM! Function signature update applied correctly.The refactor to pass the entire deployment object to
getEffectiveBuildStatusis consistent with the PR's objective to support asynchronous screenshot generation. The derivedeffectiveStatusis used correctly in the conditional logic below.src/routes/(console)/project-[region]-[project]/functions/function-[function]/deployment-[deployment]/+page.svelte (1)
43-45: LGTM! Signature updated correctly.The refactor to pass the deployment object instead of individual properties is correct and aligns with the broader changes across the codebase.
Note: Line 175 checks for
['processing', 'building']to display the spinner. If function deployments can also have a 'finalizing' state (like sites), consider adding it to this check. Verify whether function deployments require screenshot generation or if this status is site-specific only.src/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.svelte (2)
12-12: LGTM! Function signature updated correctly.The refactor to pass the deployment object is correct and consistent with the broader changes.
16-16: LGTM! 'finalizing' status handling added appropriately.Adding 'finalizing' to the status check ensures that the timer and spinner continue to display while screenshots are being generated asynchronously, which aligns with the PR's objective to prevent premature navigation to the success screen.
src/routes/(console)/project-[region]-[project]/functions/function-[function]/table.svelte (1)
97-100: LGTM! Function signature updated correctly.The refactor to pass the deployment object instead of individual status and createdAt properties is correct and aligns with the broader API changes across the codebase.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deploymentsOverview.svelte (1)
76-79: LGTM! Function signature updated correctly.The refactor to pass the deployment object directly is correct and eliminates the need for local status extraction. The effective status computation now has access to the full deployment context, which is necessary for determining the 'finalizing' state based on screenshot availability.
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.svelte (3)
8-10: LGTM!The
finalizingcase correctly falls through to return'warning', consistent with thebuildingstatus treatment.
44-44: LGTM!The call site is correctly updated to match the new
getEffectiveBuildStatussignature.
46-59: Consider handling'finalizing'status insetCopy().The
setCopy()function doesn't have a case for the new'finalizing'status. WheneffectiveStatus === 'finalizing', it will fall through to returnemptyCopy. If the intent is to show a "Finalizing deployment..." message during this phase, consider adding an explicit case.src/routes/(console)/project-[region]-[project]/sites/(components)/siteCard.svelte (1)
45-45: LGTM!The call site is correctly updated to use the new
getEffectiveBuildStatussignature.src/lib/helpers/buildTimeout.ts (1)
24-27: LGTM!The signature change to accept a
deploymentobject is a cleaner API that encapsulates related data.src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (4)
15-16: LGTM!The imports and derived
effectiveStatusare correctly set up using the updatedgetEffectiveBuildStatussignature.Also applies to: 23-23
64-68: LGTM!Proper cleanup of both the realtime subscription and interval/timeout on component unmount prevents memory leaks.
70-79: LGTM!The
goToFinishScreenfunction correctly resolves the URL and navigates to the finish page with the site ID.
119-123: LGTM!The footer text condition correctly includes the new
'finalizing'status to inform users that deployment continues in the background.
src/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.svelte
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
@src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte:
- Around line 56-59: The timeout clear is using clearInterval on
skipScreenshotTimeout even though skipScreenshotTimeout was created with
setTimeout; update the cleanup in the block that checks isReady && isFinished to
call clearTimeout(skipScreenshotTimeout) instead of clearInterval so the pending
timeout is properly cancelled before calling goToFinishScreen(); ensure
skipScreenshotTimeout references the same variable created by setTimeout.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (5)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (5)
15-16: LGTM!The imports are correctly using the
$liband$routesaliases as per coding guidelines, and are necessary for the new screenshot-waiting logic.
21-23: LGTM!The state declarations correctly use Svelte 5 runes (
$stateand$derived), and the timeout typing is appropriate.
25-68: Overall structure is solid.The realtime subscription logic correctly:
- Sets up proper cleanup on unmount (lines 64-67)
- Uses a 30-second fallback timeout to prevent indefinite waiting
- Only sets the timeout once when the deployment first becomes ready (line 50 guard)
- Updates deployment state from realtime events
The cleanup function properly handles both realtime unsubscribe and timeout cleanup.
70-79: LGTM!The
goToFinishScreenfunction correctly constructs the URL using SvelteKit'sresolve()and passes the site ID as a query parameter.
119-119: LGTM!Correctly includes
'finalizing'in the status check to display the background continuation message when the deployment is waiting for screenshots. This aligns with the neweffectiveStatusderived state.
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Outdated
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Show resolved
Hide resolved
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (2)
33-67: Good fallback mechanism with minor improvement opportunity.The 30-second fallback timeout provides good UX when screenshots are delayed. The logic correctly prevents duplicate timeouts and cleans up on success.
Consider adding a status check in the fallback timeout callback:
💡 Optional enhancement for defensive programming
if (isReady && !skipScreenshotTimeout) { skipScreenshotTimeout = setTimeout(async () => { + // Verify deployment is still ready before navigating + if (deployment.status === 'ready') { goToFinishScreen(); + } }, 30000); }This guards against the unlikely scenario where the deployment status changes after the timeout starts but before it fires.
75-84: Add error handling for navigation failures.The function uses
await goto()without error handling, which could cause silent failures or unhandled promise rejections when called from the setTimeout callback.🛡️ Add error handling
async function goToFinishScreen() { - const resolvedUrl = resolve( - '/(console)/project-[region]-[project]/sites/create-site/finish', - { - region: page.params.region, - project: page.params.project - } - ); - await goto(`${resolvedUrl}?site=${data.site.$id}`); + try { + const resolvedUrl = resolve( + '/(console)/project-[region]-[project]/sites/create-site/finish', + { + region: page.params.region, + project: page.params.project + } + ); + await goto(`${resolvedUrl}?site=${data.site.$id}`); + } catch (error) { + console.error('Failed to navigate to finish screen:', error); + // Consider showing user feedback here + } }This prevents unhandled errors from disrupting the timeout callback and provides visibility into navigation issues.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- src/routes/(console)/project-[region]-[project]/sites/(components)/logsTimer.svelte
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx,js,jsx,svelte}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,svelte}: Import reusable modules from the src/lib directory using the $lib alias
Use minimal comments in code; reserve comments for TODOs or complex logic explanations
Use $lib, $routes, and $themes aliases instead of relative paths for module imports
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte
src/routes/**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use SvelteKit file conventions: +page.svelte for components, +page.ts for data loaders, +layout.svelte for wrappers, +error.svelte for error handling, and dynamic route params in square brackets like [param]
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte
**/*.{ts,tsx,js,jsx,svelte,json}
📄 CodeRabbit inference engine (AGENTS.md)
Use 4 spaces for indentation, single quotes, 100 character line width, and no trailing commas per Prettier configuration
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/lib/helpers/buildTimeout.tssrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte
**/*.svelte
📄 CodeRabbit inference engine (AGENTS.md)
Use Svelte 5 + SvelteKit 2 syntax with TypeScript for component development
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte
src/routes/**
📄 CodeRabbit inference engine (AGENTS.md)
Configure dynamic routes using SvelteKit convention with [param] syntax in route directory names
Files:
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.sveltesrc/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.sveltesrc/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte
src/lib/helpers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Implement pure functions in src/lib/helpers/ directory and use camelCase for helper function names
Files:
src/lib/helpers/buildTimeout.ts
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
**/*.ts: Define types inline or in .d.ts files, avoid creating separate .types.ts files
Use TypeScript in non-strict mode; any type is tolerated in this project
Files:
src/lib/helpers/buildTimeout.ts
🧠 Learnings (5)
📓 Common learnings
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Capture and include UI change screenshots in PR description or comments before finalizing work
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
📚 Learning: 2025-11-19T11:22:42.553Z
Learnt from: atharvadeosthale
Repo: appwrite/console PR: 2512
File: src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte:51-83
Timestamp: 2025-11-19T11:22:42.553Z
Learning: In src/routes/(console)/project-[region]-[project]/overview/platforms/llmBanner.svelte, the Lovable integration URL format `https://lovable.dev/` with `autosubmit` and `prompt` as query parameters (set via searchParams) is correct and functional.
Applied to files:
src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to e2e/journeys/*.spec.ts : Write Playwright E2E tests in e2e/journeys/*.spec.ts files with 120s timeout and 3x retry configuration
Applied to files:
src/lib/helpers/buildTimeout.ts
📚 Learning: 2025-11-25T03:15:27.539Z
Learnt from: CR
Repo: appwrite/console PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-25T03:15:27.539Z
Learning: Applies to src/lib/helpers/*.test.ts : Write Vitest unit tests in src/lib/helpers/*.test.ts files with both client and server test setup support
Applied to files:
src/lib/helpers/buildTimeout.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: e2e
🔇 Additional comments (10)
src/routes/(console)/project-[region]-[project]/sites/(components)/logs.svelte (3)
8-10: LGTM! Correctly handles the new 'finalizing' status.The fallthrough to 'warning' appropriately treats 'finalizing' as an in-progress state, consistent with 'building'.
44-44: LGTM! Updated to match the new helper signature.Passing the entire deployment object is cleaner and allows the helper to access additional fields like screenshots.
75-75: LGTM! Simplified prop passing.LogsTimer now derives status internally from the deployment object, maintaining consistency with the refactored API.
src/routes/(console)/project-[region]-[project]/sites/site-[site]/deployments/deployment-[deployment]/+page.svelte (2)
29-29: LGTM! Consistent with the helper signature update.The call now passes the deployment object directly, aligning with the refactored API.
91-91: LGTM! Simplified LogsTimer usage.Consistent with the pattern across all deployment views.
src/lib/helpers/buildTimeout.ts (2)
24-27: LGTM! Improved API design.Accepting the full deployment object is cleaner and enables accessing additional fields like screenshots.
36-42: Consider more explicit checking for screenshot fields to handle empty strings.The truthy check
deployment.screenshotLight && deployment.screenshotDarkmay not adequately handle empty string values if these fields can contain empty strings rather than being undefined. Consider usingdeployment.screenshotLight?.length && deployment.screenshotDark?.lengthfor more explicit checking, or verify the type definitions to confirm empty strings are not possible.src/routes/(console)/project-[region]-[project]/sites/create-site/deploying/+page.svelte (3)
15-16: LGTM! Proper state management setup.The imports and state variables are correctly typed and initialized for the new screenshot-aware navigation logic.
Also applies to: 21-23
26-31: LGTM! Proper cleanup prevents memory leaks.The cleanup logic correctly clears the timeout and unsubscribes from realtime updates when the component unmounts.
Also applies to: 69-72
124-124: LGTM! Correctly includes 'finalizing' status.The message "Deployment will continue in the background" is appropriate for the finalizing state while screenshots are being generated.

What does this PR do?
Due to new Appwrite backend PR, screenshots are async, and this change prevents from going to success screen before we got screenshot.
Test Plan
✅ Manua lQA
Related PRs and Issues
appwrite/appwrite#11110
Have you read the Contributing Guidelines on issues?
Yes
Summary by CodeRabbit
New Features
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.