Conversation
![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 pull request is an automated sync that brings in a substantial number of features, refactorings, and stability improvements. Key changes include unifying the AI chat bridge into a single, more capable ACPBridge, a complete overhaul of the Tasks UI with advanced features like drag-and-drop and inline editing, and a redesigned onboarding flow. The PR also introduces significant stability enhancements, such as an auto-restart mechanism for extreme memory usage, watchdogs for subprocesses, and numerous bug fixes related to concurrency and UI. The overall quality of the changes is very high. I have one suggestion to improve maintainability by removing some duplicated code in AgentSyncService.
| } else if httpResponse.statusCode >= 500 { | ||
| let body = String(data: data, encoding: .utf8) ?? "" | ||
| log("AgentSync: push \(table) failed — HTTP \(httpResponse.statusCode): \(body)") | ||
| return .networkError // 5xx = server not ready, trigger backoff | ||
| } else { | ||
| let body = String(data: data, encoding: .utf8) ?? "" | ||
| log("AgentSync: push \(table) failed — HTTP \(httpResponse.statusCode): \(body)") |
There was a problem hiding this comment.
This new error handling logic introduces some code duplication. The lines to decode the response body and log the error are repeated in both the else if and the else blocks. This can be refactored into a single else block to handle all non-200 status codes, with a nested if for the 5xx-specific return value. This will make the code more concise and easier to maintain.
} else {
let body = String(data: data, encoding: .utf8) ?? ""
log("AgentSync: push \(table) failed — HTTP \(httpResponse.statusCode): \(body)")
if httpResponse.statusCode >= 500 {
return .networkError // 5xx = server not ready, trigger backoff
}
}
|
Hey @m13v 👋 Thank you so much for taking the time to contribute to Omi! We truly appreciate you putting in the effort to submit this pull request. After careful review, we've decided not to merge this particular PR. Please don't take this personally — we genuinely try to merge as many contributions as possible, but sometimes we have to make tough calls based on:
Your contribution is still valuable to us, and we'd love to see you contribute again in the future! If you'd like feedback on how to improve this PR or want to discuss alternative approaches, please don't hesitate to reach out. Thank you for being part of the Omi community! 💜 |
1 participant
Automated sync from omi-desktop at 474e1e1f.
This PR was created by the sync pipeline.