Merged
Conversation
Reviewer's GuideAdds a local MCP server for the QuantLab desktop with two new read-only tools for browsing and reading artifacts under outputs/, wires it into the desktop package, and surfaces pretrade artifact links in the research UI alongside a small test update and agent/editor config docs. Sequence diagram for quantlab_artifact_read MCP tool invocationsequenceDiagram
actor Developer
participant CursorClient
participant McpServer
participant NodeFs as Fs
Developer->>CursorClient: Request tool quantlab_artifact_read(relative_path)
CursorClient->>McpServer: callTool quantlab_artifact_read(relative_path)
McpServer->>McpServer: resolveOutputsPath(relative_path)
McpServer->>Fs: stat(resolvedPath)
Fs-->>McpServer: FileStat
alt path_is_directory
McpServer-->>CursorClient: isError=true, message "Expected a file under outputs/"
else binary_extension
McpServer-->>CursorClient: isError=true, message "Binary artifact reading is not supported"
else text_file
McpServer->>Fs: readFile(resolvedPath, "utf8")
Fs-->>McpServer: content
McpServer->>McpServer: build payload (bytes, modified_at, truncated, content)
McpServer-->>CursorClient: content text with metadata and artifact body
end
CursorClient-->>Developer: Display tool result
Flow diagram for outputs/ artifact listing and reading toolsflowchart TD
start([Start])
subgraph list_tool[quantlab_outputs_list]
L1{{"Tool called with optional relative_path"}}
L1 --> L2["Resolve path under outputs/ using resolveOutputsPath"]
L2 --> L3["fs.stat(targetPath)"]
L3 --> L4{Is directory?}
L4 -- No --> Lerr["Error: not a directory under outputs/"]
L4 -- Yes --> L5["fs.readdir(targetPath, withFileTypes=true)"]
L5 --> L6["For each entry: fs.stat(entryPath)"]
L6 --> L7["Build metadata: name, kind, relative_path, size_bytes, size_human, modified_at"]
L7 --> L8["Sort: directories first, then files by name"]
L8 --> L9["Return JSON payload (root, requested_path, absolute_path, entry_count, entries)"]
end
subgraph read_tool[quantlab_artifact_read]
R1{{"Tool called with relative_path"}}
R1 --> R2["Resolve path under outputs/ using resolveOutputsPath"]
R2 --> R3["fs.stat(targetPath)"]
R3 --> R4{Is directory?}
R4 -- Yes --> Rerr1["Error: expected a file under outputs/"]
R4 -- No --> R5{Binary extension?}
R5 -- Yes --> Rerr2["Error: binary artifact reading not supported"]
R5 -- No --> R6["fs.readFile(targetPath, utf8)"]
R6 --> R7["Apply truncateText to enforce MAX_OUTPUT_CHARS"]
R7 --> R8["Build response: bytes, modified_at, truncated flag, content"]
R8 --> R9["Return formatted text block with metadata and content"]
end
start --> L1
start --> R1
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The MCP tool
inputSchemadefinitions inmcp-server.mjsare currently plain objects containingz.string()etc.; consider wrapping these inz.object({...})so they match the expected Zod schema shape used by the MCP SDK and can validate inputs correctly. - The new assertion in
test_build_pretrade_handoff_payload_selects_latest_validation_artifacthardcodes a Windows-specific absolute path string, which will be brittle on non-Windows environments; consider deriving this expected value from the tmp_path/fixtures instead of embedding a platform-specific path.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The MCP tool `inputSchema` definitions in `mcp-server.mjs` are currently plain objects containing `z.string()` etc.; consider wrapping these in `z.object({...})` so they match the expected Zod schema shape used by the MCP SDK and can validate inputs correctly.
- The new assertion in `test_build_pretrade_handoff_payload_selects_latest_validation_artifact` hardcodes a Windows-specific absolute path string, which will be brittle on non-Windows environments; consider deriving this expected value from the tmp_path/fixtures instead of embedding a platform-specific path.
## Individual Comments
### Comment 1
<location path="test/test_research_ui_server.py" line_range="514-515" />
<code_context>
assert payload["handoff_id"] == "handoff-newer"
assert payload["latest_validation_path"] == str(newer)
assert payload["latest_validation_href"] == "/outputs/pretrade_handoff/newer/pretrade_handoff_validation.json"
+ assert payload["source_artifact_path"] == "C:\\Users\\marce\\Documents\\meta_trade\\tests\\fixtures\\expected_quantlab_handoff.json"
+ assert payload["source_artifact_href"] is None
</code_context>
<issue_to_address>
**issue (testing):** Avoid hard‑coding an absolute Windows path in the test to keep it portable and robust.
The test will only pass on your local Windows setup and may break if the fixture is moved. Instead of hard‑coding the full path, derive `source_artifact_path` from the same fixture/tmp directory used in setup (e.g., using `Path` joins or a helper) so the test remains portable and still verifies the field correctly.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+514
to
+515
| assert payload["source_artifact_path"] == "C:\\Users\\marce\\Documents\\meta_trade\\tests\\fixtures\\expected_quantlab_handoff.json" | ||
| assert payload["source_artifact_href"] is None |
There was a problem hiding this comment.
issue (testing): Avoid hard‑coding an absolute Windows path in the test to keep it portable and robust.
The test will only pass on your local Windows setup and may break if the fixture is moved. Instead of hard‑coding the full path, derive source_artifact_path from the same fixture/tmp directory used in setup (e.g., using Path joins or a helper) so the test remains portable and still verifies the field correctly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add two read-only MCP tools to the local QuantLab desktop server:
quantlab_outputs_listto list artifacts and directories underoutputs/quantlab_artifact_readto read text artifacts underoutputs/Scope
This is intentionally narrow:
Validation
node --check desktop/mcp-server.mjsgit diff --checkNotes
Closes #254