Codex/optimize and paginate read console tool (#511)

* Optimize read_console defaults and paging

* Fix read_console truncate test expectations

* Reduce read_console default count from 50 to 10

Further optimize token usage by reducing the default count from 50 to 10 entries. Even 10-20 messages with stack traces can be token-heavy. Added tests for default behavior and paging functionality. Updated tool description to document defaults and paging support.

* Fix ReadConsoleTests to include log type messages

The default types filter changed to ['error', 'warning'] (excluding 'log'), so tests using Debug.Log() need to explicitly request log messages. Also added format='detailed' to HandleCommand_Get_Works test since it accesses structured message fields.

* Address CodeRabbit review feedback

- Fix property naming consistency: next_cursor -> nextCursor (C# camelCase)
- Remove redundant EndGettingEntries call from catch block (already in finally)
- Extract stacktrace stripping to helper function (reduce duplication)
- Fix test mock to match actual C# response structure (items, nextCursor, truncated, total)

* perf: add early exit optimization for ReadConsole paging

- Add early exit in paging loop once page is filled, avoiding iteration
  through remaining console entries (total becomes 'at least N')
- Prefix unused mock arguments with underscores in test_read_console_truncate.py
  to suppress Ruff linter warnings

* refactor: give pageSize independent default, clarify count semantics

- Change pageSize resolution from 'pageSize ?? count ?? 50' to 'pageSize ?? 50'
  so pageSize has its own default independent of count
- count now only serves as the non-paging limit
- Add XML docs to GetConsoleEntries with clear parameter descriptions
- Update Python tool annotations to document pageSize default (50) and
  clarify that count is ignored when paging

Author: dsarno

MCPForUnity/Editor/Tools/ReadConsole.cs

@@ -165,13 +165,17 @@ public static object HandleCommand(JObject @params)
                     // Extract parameters for 'get'
                     var types =
                         (@params["types"] as JArray)?.Select(t => t.ToString().ToLower()).ToList()
-                        ?? new List<string> { "error", "warning", "log" };
+                        ?? new List<string> { "error", "warning" };
                     int? count = @params["count"]?.ToObject<int?>();
+                    int? pageSize =
+                        @params["pageSize"]?.ToObject<int?>()
+                        ?? @params["page_size"]?.ToObject<int?>();
+                    int? cursor = @params["cursor"]?.ToObject<int?>();
                     string filterText = @params["filterText"]?.ToString();
                     string sinceTimestampStr = @params["sinceTimestamp"]?.ToString(); // TODO: Implement timestamp filtering
-                    string format = (@params["format"]?.ToString() ?? "detailed").ToLower();
+                    string format = (@params["format"]?.ToString() ?? "plain").ToLower();
                     bool includeStacktrace =
-                        @params["includeStacktrace"]?.ToObject<bool?>() ?? true;
+                        @params["includeStacktrace"]?.ToObject<bool?>() ?? false;
 
                     if (types.Contains("all"))
                     {
@@ -186,7 +190,15 @@ public static object HandleCommand(JObject @params)
                         // Need a way to get timestamp per log entry.
                     }
 
-                    return GetConsoleEntries(types, count, filterText, format, includeStacktrace);
+                    return GetConsoleEntries(
+                        types,
+                        count,
+                        pageSize,
+                        cursor,
+                        filterText,
+                        format,
+                        includeStacktrace
+                    );
                 }
                 else
                 {
@@ -218,16 +230,35 @@ private static object ClearConsole()
             }
         }
 
+        /// <summary>
+        /// Retrieves console log entries with optional filtering and paging.
+        /// </summary>
+        /// <param name="types">Log types to include (e.g., "error", "warning", "log").</param>
+        /// <param name="count">Maximum entries to return in non-paging mode. Ignored when paging is active.</param>
+        /// <param name="pageSize">Number of entries per page. Defaults to 50 when omitted.</param>
+        /// <param name="cursor">Starting index for paging (0-based). Defaults to 0.</param>
+        /// <param name="filterText">Optional text filter (case-insensitive substring match).</param>
+        /// <param name="format">Output format: "plain", "detailed", or "json".</param>
+        /// <param name="includeStacktrace">Whether to include stack traces in the output.</param>
+        /// <returns>A success response with entries, or an error response.</returns>
         private static object GetConsoleEntries(
             List<string> types,
             int? count,
+            int? pageSize,
+            int? cursor,
             string filterText,
             string format,
             bool includeStacktrace
         )
         {
             List<object> formattedEntries = new List<object>();
             int retrievedCount = 0;
+            int totalMatches = 0;
+            bool usePaging = pageSize.HasValue || cursor.HasValue;
+            // pageSize defaults to 50 when omitted; count is the overall non-paging limit only
+            int resolvedPageSize = Mathf.Clamp(pageSize ?? 50, 1, 500);
+            int resolvedCursor = Mathf.Max(0, cursor ?? 0);
+            int pageEndExclusive = resolvedCursor + resolvedPageSize;
 
             try
             {
@@ -338,27 +369,39 @@ bool includeStacktrace
                             break;
                     }
 
-                    formattedEntries.Add(formattedEntry);
-                    retrievedCount++;
+                    totalMatches++;
 
-                    // Apply count limit (after filtering)
-                    if (count.HasValue && retrievedCount >= count.Value)
+                    if (usePaging)
+                    {
+                        if (totalMatches > resolvedCursor && totalMatches <= pageEndExclusive)
+                        {
+                            formattedEntries.Add(formattedEntry);
+                            retrievedCount++;
+                        }
+                        // Early exit: we've filled the page and only need to check if more exist
+                        else if (totalMatches > pageEndExclusive)
+                        {
+                            // We've passed the page; totalMatches now indicates truncation
+                            break;
+                        }
+                    }
+                    else
                     {
-                        break;
+                        formattedEntries.Add(formattedEntry);
+                        retrievedCount++;
+
+                        // Apply count limit (after filtering)
+                        if (count.HasValue && retrievedCount >= count.Value)
+                        {
+                            break;
+                        }
                     }
                 }
             }
             catch (Exception e)
             {
                 Debug.LogError($"[ReadConsole] Error while retrieving log entries: {e}");
-                // Ensure EndGettingEntries is called even if there's an error during iteration
-                try
-                {
-                    _endGettingEntriesMethod.Invoke(null, null);
-                }
-                catch
-                { /* Ignore nested exception */
-                }
+                // EndGettingEntries will be called in the finally block
                 return new ErrorResponse($"Error retrieving log entries: {e.Message}");
             }
             finally
@@ -375,6 +418,26 @@ bool includeStacktrace
                 }
             }
 
+            if (usePaging)
+            {
+                bool truncated = totalMatches > pageEndExclusive;
+                string nextCursor = truncated ? pageEndExclusive.ToString() : null;
+                var payload = new
+                {
+                    cursor = resolvedCursor,
+                    pageSize = resolvedPageSize,
+                    nextCursor = nextCursor,
+                    truncated = truncated,
+                    total = totalMatches,
+                    items = formattedEntries,
+                };
+
+                return new SuccessResponse(
+                    $"Retrieved {formattedEntries.Count} log entries.",
+                    payload
+                );
+            }
+
             // Return the filtered and formatted list (might be empty)
             return new SuccessResponse(
                 $"Retrieved {formattedEntries.Count} log entries.",

Server/src/services/tools/read_console.py

@@ -11,8 +11,15 @@
 from transport.legacy.unity_connection import async_send_command_with_retry
 
 
+def _strip_stacktrace_from_list(items: list) -> None:
+    """Remove stacktrace fields from a list of log entries."""
+    for item in items:
+        if isinstance(item, dict) and "stacktrace" in item:
+            item.pop("stacktrace", None)
+
+
 @mcp_for_unity_tool(
-    description="Gets messages from or clears the Unity Editor console. Note: For maximum client compatibility, pass count as a quoted string (e.g., '5')."
+    description="Gets messages from or clears the Unity Editor console. Defaults to 10 most recent entries. Use page_size/cursor for paging. Note: For maximum client compatibility, pass count as a quoted string (e.g., '5')."
 )
 async def read_console(
     ctx: Context,
@@ -21,10 +28,12 @@ async def read_console(
     types: Annotated[list[Literal['error', 'warning',
                                   'log', 'all']], "Message types to get"] | None = None,
     count: Annotated[int | str,
-                     "Max messages to return (accepts int or string, e.g., 5 or '5')"] | None = None,
+                     "Max messages to return in non-paging mode (accepts int or string, e.g., 5 or '5'). Ignored when paging with page_size/cursor."] | None = None,
     filter_text: Annotated[str, "Text filter for messages"] | None = None,
     since_timestamp: Annotated[str,
                                "Get messages after this timestamp (ISO 8601)"] | None = None,
+    page_size: Annotated[int | str, "Page size for paginated console reads. Defaults to 50 when omitted."] | None = None,
+    cursor: Annotated[int | str, "Opaque cursor for paging (0-based offset). Defaults to 0."] | None = None,
     format: Annotated[Literal['plain', 'detailed',
                               'json'], "Output format"] | None = None,
     include_stacktrace: Annotated[bool | str,
@@ -35,11 +44,13 @@ async def read_console(
     unity_instance = get_unity_instance_from_context(ctx)
     # Set defaults if values are None
     action = action if action is not None else 'get'
-    types = types if types is not None else ['error', 'warning', 'log']
-    format = format if format is not None else 'detailed'
+    types = types if types is not None else ['error', 'warning']
+    format = format if format is not None else 'plain'
     # Coerce booleans defensively (strings like 'true'/'false')
 
-    include_stacktrace = coerce_bool(include_stacktrace, default=True)
+    include_stacktrace = coerce_bool(include_stacktrace, default=False)
+    coerced_page_size = coerce_int(page_size, default=None)
+    coerced_cursor = coerce_int(cursor, default=None)
 
     # Normalize action if it's a string
     if isinstance(action, str):
@@ -56,7 +67,7 @@ async def read_console(
         count = coerce_int(count)
 
     if action == "get" and count is None:
-        count = 200
+        count = 10
 
     # Prepare parameters for the C# handler
     params_dict = {
@@ -65,6 +76,8 @@ async def read_console(
         "count": count,
         "filterText": filter_text,
         "sinceTimestamp": since_timestamp,
+        "pageSize": coerced_page_size,
+        "cursor": coerced_cursor,
         "format": format.lower() if isinstance(format, str) else format,
         "includeStacktrace": include_stacktrace
     }
@@ -83,16 +96,13 @@ async def read_console(
         # Strip stacktrace fields from returned lines if present
         try:
             data = resp.get("data")
-            # Handle standard format: {"data": {"lines": [...]}}
-            if isinstance(data, dict) and "lines" in data and isinstance(data["lines"], list):
-                for line in data["lines"]:
-                    if isinstance(line, dict) and "stacktrace" in line:
-                        line.pop("stacktrace", None)
-            # Handle legacy/direct list format if any
+            if isinstance(data, dict):
+                for key in ("lines", "items"):
+                    if key in data and isinstance(data[key], list):
+                        _strip_stacktrace_from_list(data[key])
+                        break
             elif isinstance(data, list):
-                for line in data:
-                    if isinstance(line, dict) and "stacktrace" in line:
-                        line.pop("stacktrace", None)
+                _strip_stacktrace_from_list(data)
         except Exception:
             pass
     return resp if isinstance(resp, dict) else {"success": False, "message": str(resp)}

Server/tests/integration/test_read_console_truncate.py

@@ -36,7 +36,7 @@ async def test_read_console_full_default(monkeypatch):
 
     captured = {}
 
-    async def fake_send(cmd, params, **kwargs):
+    async def fake_send(_cmd, params, **_kwargs):
         captured["params"] = params
         return {
             "success": True,
@@ -54,10 +54,10 @@ async def fake_send(cmd, params, **kwargs):
     resp = await read_console(ctx=DummyContext(), action="get", count=10)
     assert resp == {
         "success": True,
-        "data": {"lines": [{"level": "error", "message": "oops", "stacktrace": "trace", "time": "t"}]},
+        "data": {"lines": [{"level": "error", "message": "oops", "time": "t"}]},
     }
     assert captured["params"]["count"] == 10
-    assert captured["params"]["includeStacktrace"] is True
+    assert captured["params"]["includeStacktrace"] is False
 
 
 @pytest.mark.asyncio
@@ -67,7 +67,7 @@ async def test_read_console_truncated(monkeypatch):
 
     captured = {}
 
-    async def fake_send(cmd, params, **kwargs):
+    async def fake_send(_cmd, params, **_kwargs):
         captured["params"] = params
         return {
             "success": True,
@@ -86,3 +86,100 @@ async def fake_send(cmd, params, **kwargs):
     assert resp == {"success": True, "data": {
         "lines": [{"level": "error", "message": "oops"}]}}
     assert captured["params"]["includeStacktrace"] is False
+
+
+@pytest.mark.asyncio
+async def test_read_console_default_count(monkeypatch):
+    """Test that read_console defaults to count=10 when not specified."""
+    tools = setup_tools()
+    read_console = tools["read_console"]
+
+    captured = {}
+
+    async def fake_send(_cmd, params, **_kwargs):
+        captured["params"] = params
+        return {
+            "success": True,
+            "data": {"lines": [{"level": "error", "message": f"error {i}"} for i in range(15)]},
+        }
+
+    # Patch the send_command_with_retry function in the tools module
+    import services.tools.read_console
+    monkeypatch.setattr(
+        services.tools.read_console,
+        "async_send_command_with_retry",
+        fake_send,
+    )
+
+    # Call without specifying count - should default to 10
+    resp = await read_console(ctx=DummyContext(), action="get")
+    assert resp["success"] is True
+    # Verify that the default count of 10 was used
+    assert captured["params"]["count"] == 10
+
+
+@pytest.mark.asyncio
+async def test_read_console_paging(monkeypatch):
+    """Test that read_console paging works with page_size and cursor."""
+    tools = setup_tools()
+    read_console = tools["read_console"]
+
+    captured = {}
+
+    async def fake_send(_cmd, params, **_kwargs):
+        captured["params"] = params
+        # Simulate Unity returning paging info matching C# structure
+        page_size = params.get("pageSize", 10)
+        cursor = params.get("cursor", 0)
+        # Simulate 25 total messages
+        all_messages = [{"level": "error", "message": f"error {i}"} for i in range(25)]
+        
+        # Return a page of results
+        start = cursor
+        end = min(start + page_size, len(all_messages))
+        messages = all_messages[start:end]
+        
+        return {
+            "success": True,
+            "data": {
+                "items": messages,
+                "cursor": cursor,
+                "pageSize": page_size,
+                "nextCursor": str(end) if end < len(all_messages) else None,
+                "truncated": end < len(all_messages),
+                "total": len(all_messages),
+            },
+        }
+
+    # Patch the send_command_with_retry function in the tools module
+    import services.tools.read_console
+    monkeypatch.setattr(
+        services.tools.read_console,
+        "async_send_command_with_retry",
+        fake_send,
+    )
+
+    # First page - get first 5 entries
+    resp = await read_console(ctx=DummyContext(), action="get", page_size=5, cursor=0)
+    assert resp["success"] is True
+    assert captured["params"]["pageSize"] == 5
+    assert captured["params"]["cursor"] == 0
+    assert len(resp["data"]["items"]) == 5
+    assert resp["data"]["truncated"] is True
+    assert resp["data"]["nextCursor"] == "5"
+    assert resp["data"]["total"] == 25
+    
+    # Second page - get next 5 entries
+    resp = await read_console(ctx=DummyContext(), action="get", page_size=5, cursor=5)
+    assert resp["success"] is True
+    assert captured["params"]["cursor"] == 5
+    assert len(resp["data"]["items"]) == 5
+    assert resp["data"]["truncated"] is True
+    assert resp["data"]["nextCursor"] == "10"
+    
+    # Last page - get remaining entries
+    resp = await read_console(ctx=DummyContext(), action="get", page_size=5, cursor=20)
+    assert resp["success"] is True
+    assert len(resp["data"]["items"]) == 5
+    assert resp["data"]["truncated"] is False
+    assert resp["data"]["nextCursor"] is None