feat: web proactive-notifications (PR #4435 v2)#4845
feat: web proactive-notifications (PR #4435 v2)#4845neooriginal wants to merge 12 commits intomainfrom
Conversation
- Add proactive monitoring widget and settings - Add Gemini API client for screen analysis - Add focus analysis and proactive analysis libs - Add screen capture utilities - Integrate proactive assistant in recording UI - Add settings section for proactive configuration Cherry-picked from PR #4435 with merge conflicts resolved.
- Add clearAdviceHistory and clearFocusHistory to useProactiveNotifications - Fix confidenceThreshold access in ProactiveSettings - Fix videoElement readyState type issue in screenCapture - Add setPreviousAdvice and setFocusHistory to ProactiveContext
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- ProactiveMonitoringWidget: Change outer button to div to fix nested button HTML validation - ProactiveSettings: Fix deprecated root-level settings properties to use nested settings.advice properties Co-authored-by: neooriginal <neooriginal@users.noreply.github.com>
|
Thanks for the update, @neooriginal! If you'd like me to re-review the changes to verify the fixes, please initiate a new review by commenting |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: proactive notifications. The implementation is comprehensive, covering server actions for Gemini API communication, React contexts and hooks for state management, screen capture utilities, and UI components for settings and in-app controls. The code is generally well-structured. I've identified a few critical issues related to response validation and type correctness, as well as a high-priority security hardening opportunity. Addressing these points will improve the feature's robustness and security.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Validate nested advice object instead of top-level properties - Fix validation to check advice.advice, advice.confidence, advice.category Co-authored-by: neooriginal <neooriginal@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant new feature: proactive notifications. The implementation is comprehensive, covering everything from screen capture and AI analysis via Gemini to the user-facing UI for settings and in-app indicators. The code is well-structured with clear separation of concerns into contexts, hooks, server actions, and utility libraries. The addition of runtime validation for API responses in proactiveAnalysis.ts and focusAnalysis.ts is a great practice for robustness. My main feedback is a critical security improvement in how the Gemini API key is handled in the server action. Moving the API key from the URL to a request header is essential to minimize exposure risk. Overall, this is a solid implementation of a complex feature.
- Add ProactiveMonitoringWidget to MainLayout (floating, not inside recording) - Fix setTimeout/setInterval type definitions in screenCapture Co-authored-by: neooriginal <neooriginal@users.noreply.github.com>
This reverts commit 3f01425.
|
Hey 👋 — thanks for putting this together! Before we can review, could you share a quick live demo (screenshot, screen recording, or terminal output) showing this working on your local or dev environment? In the AI era, writing code is the easy part — what really makes a PR stand out is proof that it works end-to-end. A short video or even a screenshot goes a long way in helping reviewers feel confident about merging. Feel free to update this PR whenever you have something to show. Thanks! 🙏 |
please check refrences (PR #4435) as its just a re-factor of this |
|
Hey @neooriginal 👋 Friendly reminder — this PR has been quiet for a while. If you don't need further assistance from the CTO, feel free to go ahead and merge it. Let's close the loop and not let it go stale. Thanks! |
2 participants
Check #4435 for the description
Cherry-picked from PR #4435 with merge conflicts resolved.