feat: Add pagination support to console logs to prevent LLM token limits #42
Conversation
- Add offset and limit parameters to GetConsoleLogsResource and GetConsoleLogsTool - Implement GetLogsAsJson method with pagination in ConsoleLogsService - Return logs in newest-first order for better debugging workflow - Add automatic memory management with configurable cleanup thresholds - Update resource descriptions with pagination usage recommendations - Enhance Unity-side and Node.js-side implementations consistently 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
📝 WalkthroughWalkthroughPagination support was added to the Unity console logs retrieval pipeline across both the editor (C#) and server (JavaScript/TypeScript) layers. New methods and parameters enable clients to specify Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server (Resource/Tool)
participant Unity Editor (ConsoleLogsService)
Client->>Server (Resource/Tool): Request console logs (with offset, limit)
Server->>Unity Editor: Fetch logs (offset, limit, logType)
Unity Editor->>Unity Editor: Apply logType filter, offset, limit
Unity Editor-->>Server (Resource/Tool): Paginated logs + metadata
Server-->>Client: Paginated logs + metadata
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (2)
🧰 Additional context used🧬 Code Graph Analysis (1)Server~/src/resources/getConsoleLogsResource.ts (2)
🔇 Additional comments (9)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Nitpick comments (2)
Editor/Resources/GetConsoleLogsResource.cs (2)
29-55: Consider simplifying parameter extraction and adding validation.While the current implementation works, it could be improved for maintainability and robustness:
- string logType = null; - int offset = 0; - int limit = 100; - - if (parameters != null) - { - // Extract logType - if (parameters.ContainsKey("logType") && parameters["logType"] != null) - { - logType = parameters["logType"].ToString()?.ToLowerInvariant(); - if (string.IsNullOrWhiteSpace(logType)) - { - logType = null; - } - } - - // Extract pagination parameters - if (parameters.ContainsKey("offset") && parameters["offset"] != null) - { - int.TryParse(parameters["offset"].ToString(), out offset); - } - - if (parameters.ContainsKey("limit") && parameters["limit"] != null) - { - int.TryParse(parameters["limit"].ToString(), out limit); - } - } + string logType = parameters?["logType"]?.ToString(); + if (string.IsNullOrWhiteSpace(logType)) logType = null; + + int offset = Math.Max(0, GetIntParameter(parameters, "offset", 0)); + int limit = Math.Max(1, Math.Min(1000, GetIntParameter(parameters, "limit", 100))); + + } + + private static int GetIntParameter(JObject parameters, string key, int defaultValue) + { + if (parameters?[key] != null && int.TryParse(parameters[key].ToString(), out int value)) + return value; + return defaultValue;This approach:
- Simplifies logType extraction
- Adds bounds validation for offset (≥0) and limit (1-1000)
- Reduces code duplication with a helper method
- Maintains the same functionality with better error handling
57-67: Consider safer pagination metadata access.The response construction is well-designed, but the pagination object access could be more defensive:
- var pagination = result["pagination"] as JObject; - string typeFilter = logType != null ? $" of type '{logType}'" : ""; - result["message"] = $"Retrieved {pagination["returnedCount"]} of {pagination["filteredCount"]} log entries{typeFilter} (offset: {offset}, limit: {limit})"; + var pagination = result["pagination"] as JObject; + string typeFilter = logType != null ? $" of type '{logType}'" : ""; + int returnedCount = pagination?["returnedCount"]?.Value<int>() ?? 0; + int filteredCount = pagination?["filteredCount"]?.Value<int>() ?? 0; + result["message"] = $"Retrieved {returnedCount} of {filteredCount} log entries{typeFilter} (offset: {offset}, limit: {limit})";This protects against potential null reference exceptions if the pagination object structure changes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
Editor/Resources/GetConsoleLogsResource.cs(1 hunks)Editor/Services/ConsoleLogsService.cs(4 hunks)Editor/Services/IConsoleLogsService.cs(2 hunks)Server~/build/resources/getConsoleLogsResource.js(4 hunks)Server~/build/tools/getConsoleLogsTool.js(2 hunks)Server~/src/resources/getConsoleLogsResource.ts(5 hunks)Server~/src/tools/getConsoleLogsTool.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
Server~/build/tools/getConsoleLogsTool.js (2)
Server~/build/resources/getConsoleLogsResource.js (4)
response(70-77)logType(63-63)offset(67-67)limit(68-68)Server~/build/index.js (1)
mcpUnity(40-40)
Editor/Services/IConsoleLogsService.cs (2)
Server~/build/resources/getConsoleLogsResource.js (3)
logType(63-63)offset(67-67)limit(68-68)Editor/Services/ConsoleLogsService.cs (2)
CleanupOldLogs(164-174)GetLogCount(180-186)
Server~/build/resources/getConsoleLogsResource.js (2)
Server~/build/tools/getConsoleLogsTool.js (1)
response(61-68)Server~/build/utils/errors.js (1)
McpUnityError(10-26)
Server~/src/tools/getConsoleLogsTool.ts (3)
Server~/build/tools/getConsoleLogsTool.js (4)
toolDescription(5-5)paramsSchema(6-24)params(58-58)response(61-68)Server~/build/resources/getConsoleLogsResource.js (4)
response(70-77)logType(63-63)offset(67-67)limit(68-68)Server~/build/index.js (1)
mcpUnity(40-40)
Server~/src/resources/getConsoleLogsResource.ts (3)
Server~/build/resources/getConsoleLogsResource.js (6)
resourceUri(6-6)offset(67-67)limit(68-68)response(70-77)resourceName(4-4)logType(63-63)Server~/build/tools/getConsoleLogsTool.js (1)
response(61-68)Server~/build/index.js (1)
mcpUnity(40-40)
🔇 Additional comments (23)
Server~/build/tools/getConsoleLogsTool.js (2)
5-24: LGTM! Well-designed pagination schema with appropriate constraints.The updated tool description clearly communicates the pagination support, and the parameter schema is well-structured with:
- Proper type validation and constraints (min/max values)
- Sensible defaults (offset=0, limit=50)
- Token limit protection (max 500)
- Clear documentation for each parameter
58-67: LGTM! Clean parameter handling with consistent defaults.The destructuring assignment properly extracts the new pagination parameters with appropriate defaults that match the schema definition. The parameter passing to Unity maintains consistency with the existing pattern.
Server~/src/tools/getConsoleLogsTool.ts (2)
10-31: LGTM! Excellent TypeScript implementation with proper type safety.The parameter schema follows the same pattern as the JavaScript version with correct Zod validation. The type constraints and documentation are well-defined, maintaining consistency across the TypeScript and JavaScript implementations.
79-89: LGTM! Type-safe parameter handling with consistent implementation.The function maintains proper TypeScript typing while implementing the pagination parameters correctly. The destructuring with defaults and parameter passing aligns perfectly with the schema definition and the compiled JavaScript version.
Editor/Services/IConsoleLogsService.cs (1)
20-49: LGTM! Well-designed interface extension for pagination and memory management.The new method declarations properly support the pagination requirements:
GetLogsAsJsonprovides clean pagination with sensible defaultsCleanupOldLogsenables memory management as mentioned in PR objectivesGetLogCountsupports monitoring of log storage- Comprehensive XML documentation for all methods
- Consistent parameter naming and types
The slightly different default limit (100 vs 50 in tools) is acceptable as different layers can have different optimization strategies.
Editor/Resources/GetConsoleLogsResource.cs (1)
16-16: LGTM! Clear description with helpful performance guidance.The updated description effectively communicates the pagination support and provides practical guidance on optimal limit values for LLM token management.
Server~/build/resources/getConsoleLogsResource.js (5)
6-6: LGTM!The URI template correctly includes the pagination query parameters using standard URL query syntax.
16-16: Excellent user guidance!The updated descriptions provide clear, actionable advice about pagination parameters to avoid token limit issues. The varying recommended limits per log type (50/20/30/25) show thoughtful consideration of typical log volumes.
Also applies to: 22-22, 28-28, 34-34
46-46: Good emphasis on pagination importance.The "IMPORTANT" prefix and explicit warning about the default limit effectively communicates the necessity of using pagination parameters.
73-75: Parameters correctly passed to Unity request.The pagination parameters are properly included in the request object alongside the existing logType parameter.
83-83: Response URI correctly reflects pagination parameters.Including the actual pagination parameters in the response URI is good practice for transparency and debugging.
Server~/src/resources/getConsoleLogsResource.ts (5)
11-11: LGTM!URI template correctly includes pagination query parameters.
22-22: Clear pagination guidance provided.Descriptions effectively communicate pagination best practices and token limit concerns.
Also applies to: 28-28, 33-34, 40-40
57-57: Effective emphasis on pagination importance.Clear warning about default limit and token constraints.
87-89: Parameters correctly structured.Pagination parameters properly included in the request.
102-102: Response URI properly constructed.Correctly includes actual pagination parameters used in the request.
Editor/Services/ConsoleLogsService.cs (7)
25-26: Good memory management strategy.The constants provide reasonable limits for log retention, and the cleanup threshold approach (removing 200 entries at once) efficiently reduces cleanup frequency.
77-81: Clean refactoring to use paginated method.Good approach to maintain backward compatibility while leveraging the new paginated implementation.
83-97: Well-designed pagination API.The method signature provides sensible defaults and returns comprehensive pagination metadata.
101-146: Robust pagination implementation.The two-pass approach efficiently provides accurate counts while maintaining thread safety. The newest-first ordering aligns well with debugging workflows.
164-174: Useful manual cleanup method.Provides flexibility for users to manage log retention with an efficient bulk removal approach.
180-186: Helpful monitoring method.Simple and thread-safe way to check current log storage.
237-241: Effective automatic memory management.The cleanup logic correctly maintains the log buffer size by removing the oldest entries when the limit is exceeded.
| const offset = variables["offset"] ? parseInt(variables["offset"], 10) : 0; | ||
| const limit = variables["limit"] ? parseInt(variables["limit"], 10) : 100; |
There was a problem hiding this comment.
Add validation for pagination parameters.
The code should validate that parsed values are valid non-negative integers to prevent potential issues with NaN or negative values.
Apply this validation after parsing:
const offset = variables["offset"] ? parseInt(variables["offset"], 10) : 0;
const limit = variables["limit"] ? parseInt(variables["limit"], 10) : 100;
+
+// Validate pagination parameters
+if (isNaN(offset) || offset < 0) {
+ throw new McpUnityError(ErrorType.INVALID_PARAMS, 'Invalid offset parameter: must be a non-negative integer');
+}
+if (isNaN(limit) || limit <= 0) {
+ throw new McpUnityError(ErrorType.INVALID_PARAMS, 'Invalid limit parameter: must be a positive integer');
+}📝 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.
| const offset = variables["offset"] ? parseInt(variables["offset"], 10) : 0; | |
| const limit = variables["limit"] ? parseInt(variables["limit"], 10) : 100; | |
| const offset = variables["offset"] ? parseInt(variables["offset"], 10) : 0; | |
| const limit = variables["limit"] ? parseInt(variables["limit"], 10) : 100; | |
| // Validate pagination parameters | |
| if (isNaN(offset) || offset < 0) { | |
| throw new McpUnityError(ErrorType.INVALID_PARAMS, 'Invalid offset parameter: must be a non-negative integer'); | |
| } | |
| if (isNaN(limit) || limit <= 0) { | |
| throw new McpUnityError(ErrorType.INVALID_PARAMS, 'Invalid limit parameter: must be a positive integer'); | |
| } |
🤖 Prompt for AI Agents
In Server~/build/resources/getConsoleLogsResource.js around lines 67 to 68, the
pagination parameters offset and limit are parsed without validation, which can
lead to NaN or negative values. After parsing, add checks to ensure offset and
limit are non-negative integers; if not, set them to default values (0 for
offset, 100 for limit). This validation will prevent invalid pagination inputs
from causing errors.
…source - Add bounds validation for offset (≥0) and limit (1-1000) parameters - Simplify parameter extraction with helper method - Add defensive programming for pagination metadata access - Reduce code duplication and improve maintainability Addresses CodeRabbit review feedback for better error handling and robustness. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Editor/Resources/GetConsoleLogsResource.cs (1)
38-47: Consider adding null-safety for pagination metadata access.The response enhancement logic is comprehensive, but the pagination metadata access could be made more robust.
Consider adding explicit null checks for the pagination structure:
var pagination = result["pagination"] as JObject; string typeFilter = logType != null ? $" of type '{logType}'" : ""; -int returnedCount = pagination?["returnedCount"]?.Value<int>() ?? 0; -int filteredCount = pagination?["filteredCount"]?.Value<int>() ?? 0; +int returnedCount = pagination?["returnedCount"]?.Value<int>() ?? 0; +int filteredCount = pagination?["filteredCount"]?.Value<int>() ?? 0; + +// Additional safety check +if (pagination == null) +{ + result["message"] = $"Retrieved logs{typeFilter} (offset: {offset}, limit: {limit}) - pagination data unavailable"; + return result; +}This would handle cases where the service implementation might change or return unexpected pagination structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Editor/Resources/GetConsoleLogsResource.cs(1 hunks)
🔇 Additional comments (5)
Editor/Resources/GetConsoleLogsResource.cs (5)
16-16: Well-crafted resource description with clear guidance.The updated description effectively communicates the newest-first ordering, pagination support, and provides practical recommendations for optimal performance. The suggested limit range of 20-50 aligns well with the token limit objectives mentioned in the PR.
23-26: Documentation accurately reflects the new pagination capabilities.The method documentation has been appropriately updated to describe the pagination support and parameter expectations.
29-33: Robust parameter extraction with appropriate bounds checking.The parameter extraction logic is well-implemented:
logTypehandling correctly treats empty/whitespace strings as nulloffsetbounds ensure non-negative values withMath.Max(0, ...)limitbounds ensure values between 1-1000 with nestedMath.Max/Mincalls- Default values (offset=0, limit=100) are sensible for the use case
36-36: Method call correctly updated to support pagination.The change from
GetAllLogsAsJson(logType)toGetLogsAsJson(logType, offset, limit)appropriately utilizes the new paginated service method.
50-62: Excellent helper method implementation with proper safety measures.The
GetIntParameterhelper method demonstrates good practices:
- Uses
TryParsefor safe integer conversion- Includes proper null checking with the null-conditional operator
- Returns sensible defaults on parsing failures
private staticmodifier provides appropriate encapsulation
Resolved conflict by combining pagination functionality with the new log type mapping feature from upstream.
| string typeFilter = logType != null ? $" of type '{logType}'" : ""; | ||
| int returnedCount = pagination?["returnedCount"]?.Value<int>() ?? 0; | ||
| int filteredCount = pagination?["filteredCount"]?.Value<int>() ?? 0; | ||
| result["message"] = $"Retrieved {returnedCount} of {filteredCount} log entries{typeFilter} (offset: {offset}, limit: {limit})"; |
There was a problem hiding this comment.
change the return to be the same JObject format as before
is a more readable and direct text for the AI agent to process
| @@ -79,10 +83,26 @@ public void StopListening() | |||
| /// </summary> | |||
| /// <returns>JArray containing all logs</returns> | |||
| public JArray GetAllLogsAsJson(string logType = "") | |||
| @@ -101,21 +121,52 @@ public JArray GetAllLogsAsJson(string logType = "") | |||
|
|
|||
| lock (_logEntries) | |||
| { | |||
| // First pass: count total and filtered entries | |||
| foreach (var entry in _logEntries) | |||
There was a problem hiding this comment.
change to one single for loop for efficiency
if there is a limit and offset, that can be counted in the for loop variables
| return new JObject | ||
| { | ||
| ["logs"] = logsArray, | ||
| ["pagination"] = new JObject |
There was a problem hiding this comment.
pagination is unecessary
the message log above is already complete to be fullfiled with the total logs count
| @@ -19,25 +19,25 @@ function listLogTypes(resourceMimeType: string) { | |||
| { | |||
| uri: `unity://logs/`, | |||
There was a problem hiding this comment.
for the resource update the template with default offset and limit values
use max values that can be accepted
do this to all template commands
|
@Saqoosha PR reviewed |
- Remove pagination object and consolidate info into message field - Delete deprecated GetAllLogsAsJson method - Optimize log counting with single loop for better performance - Add default offset/limit values to resource templates - Simplify response to only include logs, message, and success fields - Move count information (total, filtered, returned) into message text - Add parameter validation for offset/limit in TypeScript This addresses all review comments from PR CoderGamester#42 to make the response format more concise and easier for AI agents to process. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <[email protected]>
|
Hi @CoderGamester, Thanks for the review! I've addressed all your requested changes:
The latest commit (5e35e08) contains all these changes. The pagination feature has been tested and is working properly with all log types. Ready for your review! |
|
@Saqoosha thank you very much for the PR |
🚨 Problem
When Unity console logs accumulate in large quantities (hundreds to thousands of entries), the
get_console_logsfunction fails due to LLM token limit exceeded errors."response (27806 tokens) exceeds maximum allowed tokens (25000)"⚡ Solution
Pagination Implementation
offsetandlimitparameters for chunked log retrievalMemory Management Enhancement
Consistent Implementation Across Both Sides
GetLogsAsJson(logType, offset, limit)method📊 Results
Dramatic Token Usage Reduction
Recommended Usage Examples
Performance Improvements
🧪 Testing
Thoroughly tested with large volumes of test logs (Error/Exception/Assert/Warning/Info):
📋 Technical Details
Unity Side Changes
Editor/Services/ConsoleLogsService.cs: Added paginatedGetLogsAsJsonmethodEditor/Services/IConsoleLogsService.cs: Extended interface with pagination supportEditor/Resources/GetConsoleLogsResource.cs: Added parameter extraction and processingNode.js Side Changes
Server~/src/tools/getConsoleLogsTool.ts: Added offset/limit parameters to Zod schemaServer~/src/resources/getConsoleLogsResource.ts: Extended URL template to support paginationKey Features
get_console_logscalls continue to work unchangedBreaking Changes: None (fully backward compatible)
🔍 Additional Implementation Details
Pagination Metadata Structure
The response now includes a detailed pagination object with:
Memory Management Constants
MaxLogEntries: 1000 (maximum logs stored in memory)CleanupThreshold: 200 (number of oldest entries removed when exceeding max)Resource URI Updates
Updated resource URI template to support pagination parameters:
Enhanced Documentation
Created with Palmier
Summary by CodeRabbit
New Features
Improvements