Add structured per-monitor display inventory by zengfanfan · Pull Request #329 · CursorTouch/Windows-MCP

zengfanfan · 2026-07-11T13:10:58Z

Summary

add a read-only DisplayInventory tool for active Windows monitors
return a native top-level array with stable active-display indices, device names, virtual-screen bounds, work areas, resolution, orientation, effective DPI, and scale
reuse the existing monitor enumeration path and preserve negative virtual-screen coordinates
read current display mode through EnumDisplaySettingsW
use per-monitor effective DPI with a system-DPI fallback when the per-monitor API is unavailable

Why

Agents can select displays for capture, but they cannot currently inspect the monitor layout and DPI metadata before choosing coordinates or validating a DPI-specific run. A structured inventory makes that state explicit without changing existing screenshot or input behavior.

Output shape

[
  {
    "index": 0,
    "device": "\\\\.\\DISPLAY1",
    "primary": true,
    "bounds": {"left": 0, "top": 0, "right": 1920, "bottom": 1080, "width": 1920, "height": 1080},
    "work_area": {"left": 0, "top": 0, "right": 1920, "bottom": 1040, "width": 1920, "height": 1040},
    "resolution": "1920x1080",
    "orientation": "landscape",
    "effective_dpi": 96,
    "scale": 1.0
  }
]

Duplicate and overlap check

PR Add coordinate_system parameter for DPI-aware coordinate conversion #81 changes input coordinate conversion; it does not expose display inventory.
PR feat: add 9 advanced automation tools with DPI coordinate system support #93 proposes a single ScreenInfo summary; it does not expose per-monitor work areas, orientation, or effective DPI records.
No equivalent implementation exists on current main.

Testing

ruff check on all changed Python files: passed
ruff format --check on all changed Python files: passed
focused display tests: 4 passed
full suite: 322 passed, 3 failed
the same three tests/test_iserror_compliance.py import failures reproduce on unmodified upstream/main
current real Windows 10 smoke inventory: 1920x1080, DPI 96, scale 1.0, landscape
the per-monitor DPI path was also manually validated at real 150% scaling before this clean branch split

Dependencies

None. This branch is based directly on current main and does not depend on the deterministic-drag PR.

Closes #327

qodo-code-review · 2026-07-11T15:01:48Z

PR Summary by Qodo

Add DisplayInventory tool for per-monitor layout, work area, and DPI metadata

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Add a read-only DisplayInventory tool that returns active monitor layout and DPI metadata.
• Extend monitor enumeration to include work area, effective DPI/scale, and orientation.
• Add focused tests for tool output, DPI fallback behavior, and orientation detection.

Diagram

graph TD
  A["MCP Client"] --> B["DisplayInventory tool"] --> C["Desktop.get_displays()"] --> D["uia.core monitor scan"]
  D --> E["Win32 DPI APIs"]
  D --> F["EnumDisplaySettingsW"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Use typed ctypes structures for DEVMODEW (avoid manual offsets)

➕ Less brittle than hard-coded byte offsets when reading width/height
➕ Self-documenting field names reduce maintenance risk
➖ More code to define/maintain structs across 32/64-bit and packing nuances
➖ Still depends on Win32 API availability/behavior

2. Use QueryDisplayConfig for orientation/mode metadata

➕ More authoritative for modern display topology and rotation state
➕ Potentially richer metadata (paths/modes) for future extensions
➖ Significantly more complex API surface and parsing logic
➖ May require additional compatibility handling across Windows versions

Recommendation: Current approach is reasonable for a lightweight, read-only inventory tool: it reuses the existing monitor enumeration path and adds pragmatic DPI/orientation enrichment with safe fallbacks. If this tool becomes heavily relied upon, consider replacing the DEVMODE byte-offset parsing with a proper ctypes DEVMODEW struct first (lowest-risk hardening), and only move to QueryDisplayConfig if richer topology data is needed.

Files changed (5) +263 / -0

Enhancement (2) +155 / -0

display.pyAdd DisplayInventory MCP tool implementation +66/-0

Add DisplayInventory MCP tool implementation

• Introduces a read-only 'DisplayInventory' tool that returns a JSON payload with display index, device name, bounds, work area, resolution, orientation, effective DPI, and scale. Includes a helper to serialize Rect-like objects while preserving negative virtual-screen coordinates.

src/windows_mcp/tools/display.py

core.pyEnrich DisplayInfo with work area, DPI/scale, and orientation +89/-0

Enrich DisplayInfo with work area, DPI/scale, and orientation

• Extends 'DisplayInfo' to include 'work_rect', 'effective_dpi', 'scale', and 'orientation'. Augments monitor enumeration to populate work area, compute effective per-monitor DPI with a system-DPI fallback, and infer orientation using 'EnumDisplaySettingsW' with bounds-based fallback.

src/windows_mcp/uia/core.py

Tests (1) +105 / -0

test_display_inventory_tool.pyAdd tests for DisplayInventory output and DPI/orientation fallbacks +105/-0

Add tests for DisplayInventory output and DPI/orientation fallbacks
• Adds unit tests validating the tool’s JSON output shape, DPI fallback behavior when per-monitor DPI is unavailable, and orientation inference using current display mode with a bounds fallback.
tests/test_display_inventory_tool.py

Documentation (1) +1 / -0

README.mdDocument new DisplayInventory tool in tool list +1/-0

Document new DisplayInventory tool in tool list

• Adds 'DisplayInventory' to the README’s list of MCP tools and briefly describes the metadata it returns (layout, work areas, effective DPI, scale).

README.md

Other (1) +2 / -0

__init__.pyRegister the new display tool module +2/-0

Register the new display tool module
• Imports the new 'display' tool module and adds it to the tool module registration list so it is loaded by the server.
src/windows_mcp/tools/init.py

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

qodo-code-review · 2026-07-11T15:07:04Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (3) 📎 Requirement gaps (1) 📜 Skill insights (0)

Context used

✅ Tickets: 🎫 Proposal: expose structured per-monitor DPI inventory

✅ Compliance rules (platform): 15 rules

1. DisplayInventory not JSON array 📎 Requirement gap ≡ Correctness

Description

The DisplayInventory tool returns an object with displays/count instead of a top-level JSON
array of per-monitor records. This violates the required output contract and may break clients
expecting an array response.

Code

src/windows_mcp/tools/display.py[R49-66]

+        payload = {
+            "displays": [
+                {
+                    "index": display.index,
+                    "device": display.device_name,
+                    "primary": display.primary,
+                    "bounds": _rect_to_dict(display.rect),
+                    "work_area": _rect_to_dict(getattr(display, "work_rect", None)),
+                    "resolution": f"{display.rect.width()}x{display.rect.height()}",
+                    "orientation": getattr(display, "orientation", None),
+                    "effective_dpi": getattr(display, "effective_dpi", None),
+                    "scale": getattr(display, "scale", None),
+                }
+                for display in displays
+            ],
+            "count": len(displays),
+        }
+        return json.dumps(payload, indent=2)

Relevance

⭐⭐ Medium
No historical evidence enforcing array-only DisplayInventory output contract.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 222811 requires a DisplayInventory tool that returns a JSON array of per-monitor
objects. The implementation instead returns json.dumps(payload) where payload is a dict
containing displays and count.
Expose read-only DisplayInventory tool with per-active-monitor structured output
src/windows_mcp/tools/display.py[49-66]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`DisplayInventory` currently returns a JSON object containing `displays` and `count`, but the compliance requirement specifies that the tool must return a top-level JSON array with one object per active monitor.

## Issue Context
Keeping a stable tool response shape matters for MCP clients. If `count` is still desired, consider adding it via a separate tool or embedding it in each record is not appropriate; the rule explicitly calls for a top-level array.

## Fix Focus Areas
- src/windows_mcp/tools/display.py[49-66]
- tests/test_display_inventory_tool.py[40-75]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Long assert line 📘 Rule violation ✧ Quality

Description

A newly added test assertion appears to exceed the 100 character maximum line length. This violates
the style requirement and can reduce readability in reviews and diffs.

Code

tests/test_display_inventory_tool.py[98]

+    assert core._get_display_orientation("\\\\.\\DISPLAY1", Rect(0, 0, 1080, 1920)) == "portrait"

Relevance

⭐⭐⭐ High
Line-length/style fixes in tests accepted recently (PR #307).
PR-#307

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 222796 enforces a maximum line length of 100 characters for non-comment code lines.
The added assertion line is long enough to exceed this limit and should be wrapped.
Rule 222796: Enforce maximum line length of 100 characters
tests/test_display_inventory_tool.py[98-98]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added line exceeds the 100 character maximum.

## Issue Context
Wrap the assertion using parentheses and line breaks at natural boundaries (e.g., after commas) to keep each line within 100 characters.

## Fix Focus Areas
- tests/test_display_inventory_tool.py[98-98]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. hMonitor not snake_case 📘 Rule violation ✧ Quality

Description

The new function parameter hMonitor uses mixed casing rather than snake_case. This reduces
consistency with Python naming conventions required by the checklist.

Code

src/windows_mcp/uia/core.py[R711-717]

+def _get_monitor_effective_dpi(hMonitor: int) -> tuple[int | None, float | None]:
+    try:
+        dpi_x = ctypes.c_uint()
+        dpi_y = ctypes.c_uint()
+        result = ctypes.windll.shcore.GetDpiForMonitor(
+            ctypes.c_void_p(hMonitor),
+            0,

Relevance

⭐⭐ Medium
No past reviews found about renaming hMonitor-style Win32 handle params.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 222797 requires snake_case for function/variable identifiers. The new parameter
hMonitor in _get_monitor_effective_dpi is not snake_case.
Rule 222797: Enforce snake_case for function and variable identifiers
src/windows_mcp/uia/core.py[711-717]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
A newly added identifier (`hMonitor`) violates the snake_case naming requirement for function/variable identifiers.

## Issue Context
This is a Windows handle parameter, but the rule still requires snake_case in Python code. Rename consistently and update all internal references.

## Fix Focus Areas
- src/windows_mcp/uia/core.py[711-726]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

4. Pre-serialized inventory output 🐞 Bug ⚙ Maintainability

Description

DisplayInventory returns a JSON-formatted string via json.dumps, making the tool result text-only
rather than a native structured response like Snapshot/Screenshot; this forces consumers to
JSON-parse manually and makes tool outputs inconsistent across the toolset.

Code

src/windows_mcp/tools/display.py[R49-66]

+        payload = {
+            "displays": [
+                {
+                    "index": display.index,
+                    "device": display.device_name,
+                    "primary": display.primary,
+                    "bounds": _rect_to_dict(display.rect),
+                    "work_area": _rect_to_dict(getattr(display, "work_rect", None)),
+                    "resolution": f"{display.rect.width()}x{display.rect.height()}",
+                    "orientation": getattr(display, "orientation", None),
+                    "effective_dpi": getattr(display, "effective_dpi", None),
+                    "scale": getattr(display, "scale", None),
+                }
+                for display in displays
+            ],
+            "count": len(displays),
+        }
+        return json.dumps(payload, indent=2)

Relevance

⭐⭐ Medium
No past reviews found about avoiding json.dumps in tool outputs.
ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The tool explicitly returns json.dumps(...) (string). Snapshot/Screenshot paths demonstrate the
codebase already supports returning structured content (a list with text/image parts), highlighting
the inconsistency.
src/windows_mcp/tools/display.py[47-66]
src/windows_mcp/tools/_snapshot_helpers.py[134-218]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`DisplayInventory` currently returns `json.dumps(payload, indent=2)` (a string). This makes the tool output inconsistent with other tools that return structured content (e.g., Snapshot returns a list of content parts), and it requires clients/agents to manually parse JSON to use the data programmatically.

### Issue Context
The tool already builds a fully JSON-serializable `payload` dict; the only question is the most appropriate return type for FastMCP/MCP consumption.

### Fix Focus Areas
- src/windows_mcp/tools/display.py[47-66]

### Suggested fix
Return `payload` directly (or, if the MCP framework expects content parts, return `[json.dumps(payload)]` consistently with other text-returning tools). Ensure tests are updated to match the chosen return type.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. register missing docstring 📘 Rule violation ✧ Quality

Description

The new public function register has no Google-style docstring. This reduces API clarity for a
tool registration entry point.

Code

src/windows_mcp/tools/display.py[R25-30]

+def register(
+    mcp: Any,
+    *,
+    get_desktop: Callable[[], Any],
+    get_analytics: Callable[[], Any],
+) -> None:

Relevance

⭐ Low
Docstring-to-Google-style enforcement previously rejected (PRs #202, #232).
PR-#202
PR-#232

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
PR Compliance ID 222802 requires Google-style docstrings on public functions/classes in changed
files. register is a public function (no leading underscore) and has no docstring immediately
under its definition.
Rule 222802: Require Google-style docstrings on public functions and classes
src/windows_mcp/tools/display.py[25-30]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The public `register` function is missing a Google-style docstring describing purpose, arguments, and behavior.

## Issue Context
This module defines an MCP tool. The registration entry point is part of the module's public surface and should be documented per the checklist.

## Fix Focus Areas
- src/windows_mcp/tools/display.py[25-30]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan

Jeomon · 2026-07-13T06:30:36Z

Thanks

zengfanfan added 2 commits July 11, 2026 21:10

Add per-monitor display inventory

c949a33

Document display inventory tool

d9d6a08

zengfanfan mentioned this pull request Jul 11, 2026

Proposal: expose structured per-monitor DPI inventory #327

Closed

zengfanfan marked this pull request as ready for review July 11, 2026 15:00

qodo-code-review Bot reviewed Jul 11, 2026

View reviewed changes

Comment thread src/windows_mcp/tools/display.py Outdated

Comment thread src/windows_mcp/uia/core.py Outdated

Comment thread tests/test_display_inventory_tool.py

Comment thread src/windows_mcp/tools/display.py Outdated

Address display inventory review findings

5da5151

Jeomon merged commit 9c9834d into CursorTouch:main Jul 13, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add structured per-monitor display inventory#329

Add structured per-monitor display inventory#329
Jeomon merged 3 commits into
CursorTouch:mainfrom
zengfanfan:codex/upstream-display-inventory

zengfanfan commented Jul 11, 2026 •

edited

Loading

Uh oh!

qodo-code-review Bot commented Jul 11, 2026

Uh oh!

qodo-code-review Bot commented Jul 11, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jeomon commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

zengfanfan commented Jul 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Output shape

Duplicate and overlap check

Testing

Dependencies

Uh oh!

qodo-code-review Bot commented Jul 11, 2026

PR Summary by Qodo

Uh oh!

qodo-code-review Bot commented Jul 11, 2026

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Jeomon commented Jul 13, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

zengfanfan commented Jul 11, 2026 •

edited

Loading