⚡ Bolt: Optimize psutil process scans by lazily evaluating expensive properties#17
⚡ Bolt: Optimize psutil process scans by lazily evaluating expensive properties#17inherent-vice wants to merge 1 commit into
Conversation
Co-authored-by: agno7766 <125467265+agno7766@users.noreply.github.com>
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
📝 WalkthroughWalkthroughThis PR optimizes process scanning in two desktop services by deferring expensive ChangesDesktop process scanning optimization
🎯 2 (Simple) | ⏱️ ~12 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.jules/bolt.md:
- Line 1: Update the changelog header "## 2024-05-18 - [Optimize
psutil.process_iter scans]" to use the PR creation year 2026 by changing it to
"## 2026-05-18 - [Optimize psutil.process_iter scans]" so the entry date matches
the PR; edit the header line in .jules/bolt.md (the markdown heading string
shown in the diff) to reflect the corrected year.
In `@conftest.py`:
- Line 13: The test setup currently injects a MagicMock into
sys.modules['ctypes.windll'] which doesn't cover attribute access patterns like
import ctypes; ctypes.windll.*; update the mock so that when sys.platform !=
'win32' you import ctypes and assign a MagicMock to the ctypes.windll attribute
(i.e., replace the sys.modules assignment with setting ctypes.windll =
MagicMock()), ensuring code that does ctypes.windll.<name> will be intercepted;
keep the platform check and use the existing MagicMock instance name.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 61628e6e-1256-4baf-908a-dd791c52b62f
📒 Files selected for processing (5)
.jules/bolt.mdconftest.pydesktop_services/excel_service.pydesktop_services/operations_cockpit_service.pytests/test_excel_desktop_service.py
| @@ -0,0 +1,3 @@ | |||
| ## 2024-05-18 - [Optimize psutil.process_iter scans] | |||
There was a problem hiding this comment.
Correct the date to match the PR creation date.
The documentation entry is dated "2024-05-18" but this PR was created on 2026-05-18. The year should be 2026 to accurately reflect when this optimization was implemented.
📅 Proposed fix for the date
-## 2024-05-18 - [Optimize psutil.process_iter scans]
+## 2026-05-18 - [Optimize psutil.process_iter scans]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## 2024-05-18 - [Optimize psutil.process_iter scans] | |
| ## 2026-05-18 - [Optimize psutil.process_iter scans] |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.jules/bolt.md at line 1, Update the changelog header "## 2024-05-18 -
[Optimize psutil.process_iter scans]" to use the PR creation year 2026 by
changing it to "## 2026-05-18 - [Optimize psutil.process_iter scans]" so the
entry date matches the PR; edit the header line in .jules/bolt.md (the markdown
heading string shown in the diff) to reflect the corrected year.
| sys.modules['pywintypes'] = MagicMock() | ||
| sys.modules['win32api'] = MagicMock() | ||
| sys.modules['win32con'] = MagicMock() | ||
| sys.modules['ctypes.windll'] = MagicMock() |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check how ctypes.windll is imported and used in desktop services
# Search for ctypes imports and usage patterns
rg -n -C3 --type=py 'ctypes\.windll|from ctypes import windll|import ctypes\.windll' desktop_services/Repository: inherent-vice/WindowManager
Length of output: 5482
Fix ctypes.windll mocking to intercept attribute access, not just module imports.
The current sys.modules['ctypes.windll'] = MagicMock() approach only intercepts import ctypes.windll statements. However, the codebase uses the pattern import ctypes; ctypes.windll.XXX (e.g., ctypes.windll.shell32.ShellExecuteW), which accesses windll as an attribute on the imported ctypes module object. This mocking won't prevent an AttributeError on non-Windows platforms where the windll attribute doesn't exist.
Replace with:
if sys.platform != 'win32':
import ctypes
ctypes.windll = MagicMock()🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@conftest.py` at line 13, The test setup currently injects a MagicMock into
sys.modules['ctypes.windll'] which doesn't cover attribute access patterns like
import ctypes; ctypes.windll.*; update the mock so that when sys.platform !=
'win32' you import ctypes and assign a MagicMock to the ctypes.windll attribute
(i.e., replace the sys.modules assignment with setting ctypes.windll =
MagicMock()), ensuring code that does ctypes.windll.<name> will be intercepted;
keep the platform check and use the existing MagicMock instance name.
![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.
Reviewed commit: 1c7764dab7
ℹ️ 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 "@codex address that feedback".
| sys.modules['win32com.client'] = MagicMock() | ||
| sys.modules['win32gui'] = MagicMock() | ||
| sys.modules['win32process'] = MagicMock() | ||
| sys.modules['pywintypes'] = MagicMock() |
There was a problem hiding this comment.
On non-Windows pytest runs this replaces pywintypes with a bare MagicMock, so from pywintypes import com_error yields another mock rather than an exception class. The Excel COM tests and excel_instance_manager use com_error(...) as an exception and catch it in except com_error; with this stub, side_effect is treated as a callable instead of raising, and any real exception through those handlers becomes TypeError: catching classes that do not inherit from BaseException is not allowed. The stub should expose a real Exception subclass for com_error instead of a generic mock.
Useful? React with 👍 / 👎.
1 participant
💡 What: Updated
psutil.process_iter()usage inExcelDesktopService._get_excel_process_snapshotsandExcelSessionGuard._default_processes. Instead of querying["pid", "name", "memory_info", "status"]for every system process, it now only queries["pid", "name"]initially. The expensivememory_info()andstatus()properties are then fetched only for actual "excel.exe" processes.🎯 Why: Querying full
memory_infofor all system processes causes significant overhead because the OS has to evaluate the memory details of hundreds of unneeded processes.📊 Impact: Reduces system overhead drastically during process scans. Scanning lightweight processes only drops overhead from evaluating ~200+ processes to just the active Excel ones (typically 1-5).
🔬 Measurement: Verify by reviewing process query times or utilizing profiling traces on
get_running_instancesto see decreased latency during refreshes.PR created automatically by Jules for task 14323550085384020067 started by @agno7766
Summary by CodeRabbit
Bug Fixes
Refactor