Skip to content

Comments

fix(cli): enhance backend to send the partial application#150

Merged
davemooreuws merged 1 commit intomainfrom
partial-sync
Dec 4, 2025
Merged

fix(cli): enhance backend to send the partial application#150
davemooreuws merged 1 commit intomainfrom
partial-sync

Conversation

@raksiv
Copy link
Member

@raksiv raksiv commented Nov 28, 2025

Allows the frontend to preserve existing fields when errors occur during application YAML parsing or validation.

This is to accomodate a new screen which prompts you to pick a platform when none is configured e.g. when you use suga init.

image

Allows the frontend to preserve existing fields when errors occur during application YAML parsing or validation.
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Nov 28, 2025

📝 Walkthrough

Walkthrough

Well ackchyually… the changes modify error handling in the file synchronization logic. The getApplicationFileContents function now returns a partial application with errors instead of nil when YAML schema validation fails. The OnConnect and watchFile functions are updated to compute validation errors during failures and broadcast partial applications via syncMessage before sending syncError messages with validation details. This approach preserves existing frontend fields when errors occur during parsing or validation.

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The description explains the purpose of the changes (preserving frontend fields during errors) and the use case (new platform selection screen), which aligns with the changeset.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Title check ✅ Passed The title clearly and concisely describes the main change: enhancing the backend to send partial applications during errors.

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
cli/internal/devserver/filesync.go (1)

213-214: Well ackchyually… file watching stops on any error, breaking auto-reload.

The fileError variable captures errors from the debounced handler, then Line 253 returns it, exiting watchFile entirely. If a YAML parse error occurs, file watching stops permanently. When the user fixes the error, the devserver won't detect the change.

Remove the error return to keep watching:

-			var fileError error = nil
 			debounced, cancel = lo.NewDebounce(fs.debounce, func() {
 				application, contents, err := fs.getApplicationFileContents()
 				if err != nil {
 					// Update contents so sync can be handled after validation errors clear
 					fs.lastSyncContents = contents
 
-					fileError = err
-
 					validationErrors, validationErr := validateApplicationSchema(contents)
 					if validationErr != nil {
 						return
 					}
 
 					// Send partial application data first so frontend has existing fields to merge with
 					if application != nil {
 						fs.broadcast(Message[any]{
 							Type:    "syncMessage",
 							Payload: *application,
 						})
 					}
 
 					fs.broadcast(Message[any]{
 						Type:    "syncError",
 						Payload: validationErrors,
 					})
 					return
 				}
 
 				if bytes.Equal(fs.lastSyncContents, contents) {
 					return
 				}
 
 				fs.broadcast(Message[any]{
 					Type:    "syncMessage",
 					Payload: *application,
 				})
 			})
 			debounced()
-			if fileError != nil {
-				return fileError
-			}
 		}
 	}

Also applies to: 252-254

♻️ Duplicate comments (1)
cli/internal/devserver/filesync.go (1)

223-225: Well ackchyually… same silent failure here too.

Identical to Line 100-102: if validation error computation fails, the debounced function returns without notifying clients.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 49831e9 and e63dd8d.

📒 Files selected for processing (1)
  • cli/internal/devserver/filesync.go (3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
cli/internal/devserver/filesync.go (1)
cli/internal/devserver/devserver.go (1)
  • Message (16-19)
⏰ 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). (1)
  • GitHub Check: Build (macos-latest, arm64)
🔇 Additional comments (4)
cli/internal/devserver/filesync.go (4)

51-59: LGTM! Partial application preservation enables better UX.

The changes correctly return the partial application alongside errors, allowing the frontend to preserve existing fields during validation failures. The error message on Line 58 is generic but sufficient since detailed validation errors are computed separately in validateApplicationSchema.


78-94: LGTM! Clean validation error computation.

The helper function correctly aggregates schema validation errors and application spec errors into a single slice.


105-110: LGTM! Partial application sent before errors enables frontend field preservation.

The nil check and ordering are correct for the intended UX flow.


228-233: LGTM! Consistent with OnConnect pattern.

Correctly sends partial application before validation errors, maintaining the intended sequence for frontend reconciliation.

@raksiv raksiv changed the title fix: enhance backend to send the partial application fix(cli): enhance backend to send the partial application Nov 28, 2025
@davemooreuws davemooreuws merged commit d9d2748 into main Dec 4, 2025
12 of 13 checks passed
@davemooreuws davemooreuws deleted the partial-sync branch December 4, 2025 00:35
@nitric-bot
Copy link

🎉 This PR is included in version 0.6.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants