feat(research-ui): surface pretrade artifact paths#252
Merged
Conversation
Reviewer's GuideAdds read-only display of pre-trade validation and source artifact paths to the research UI pre-trade panel, wiring through the latest validation and source artifact metadata from the server and covering it with focused tests. Sequence diagram for loading pretrade artifact metadata into the UIsequenceDiagram
actor Operator
participant Browser_research_ui
participant Research_ui_server
participant Pretrade_service
Operator->>Browser_research_ui: Select pretrade for review
Browser_research_ui->>Research_ui_server: GET /api/pretrade/{id}
Research_ui_server->>Pretrade_service: Fetch pretrade decision state
Pretrade_service-->>Research_ui_server: pretrade core fields
Research_ui_server->>Pretrade_service: Fetch artifact metadata
Pretrade_service-->>Research_ui_server: latest_validation_path, latest_validation_href, source_artifact_path, source_artifact_href
Research_ui_server-->>Browser_research_ui: pretrade JSON with artifact fields
Browser_research_ui->>Browser_research_ui: buildPretradePanel(pretrade)
Browser_research_ui->>Browser_research_ui: Render artifactLinks list or empty placeholder
Operator-->>Browser_research_ui: Optionally click validation artifact link
Browser_research_ui-->>Operator: Open validation artifact in new tab
Flow diagram for rendering artifact links in buildPretradePanelflowchart TD
A["Start buildPretradePanel"] --> B["Initialize artifactEntries with validation and source artifacts"]
B --> C{"artifact.path or artifact.href for any entry?"}
C -->|No| D["Render panel-empty compact-empty with No artifact paths yet message"]
C -->|Yes| E["Filter artifactEntries to those with path or href"]
E --> F["Map each artifact to DOM fragment"]
F --> G{"artifact.href present?"}
G -->|Yes| H["Render a.artifact-link with label and path, target _blank"]
G -->|No| I["Render div.artifact-link with label and path_or_fallback"]
H --> J["Join fragments into artifact-list container"]
I --> J
J --> K["Inject artifactLinks HTML into pretrade panel below key-value grid"]
K --> L["Render reasons inline-list"]
L --> M["End buildPretradePanel"]
File-Level Changes
Assessment against 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
artifactEntriesfilter drops artifacts that have neitherpathnorhref, which means the per-artifactfallbackmessages are never shown; consider removing or adjusting the filter so each artifact row can display its specific fallback copy instead of only the global empty state. - The test
test_build_pretrade_handoff_payload_selects_latest_validation_artifacthardcodes an absolute Windows-specific source path (C:\Users\marce\...), which will be brittle and non-portable; derive this path from a fixture ortmp_pathinstead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `artifactEntries` filter drops artifacts that have neither `path` nor `href`, which means the per-artifact `fallback` messages are never shown; consider removing or adjusting the filter so each artifact row can display its specific fallback copy instead of only the global empty state.
- The test `test_build_pretrade_handoff_payload_selects_latest_validation_artifact` hardcodes an absolute Windows-specific source path (`C:\Users\marce\...`), which will be brittle and non-portable; derive this path from a fixture or `tmp_path` instead.
## 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 asserting on a hard-coded, absolute, Windows-specific path in the test
This hard-coded, user-specific Windows path will make the test fail on CI and on non-Windows or differently configured machines. Instead, derive the expected value from the test paths/fixtures already in use (e.g., via `tmp_path` or a known fixture root) and compare a constructed or normalized path (using `os.path.join` / `os.path.normpath`, or checking a relative suffix) so the test validates the contract without depending on a specific machine layout.
</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 asserting on a hard-coded, absolute, Windows-specific path in the test
This hard-coded, user-specific Windows path will make the test fail on CI and on non-Windows or differently configured machines. Instead, derive the expected value from the test paths/fixtures already in use (e.g., via tmp_path or a known fixture root) and compare a constructed or normalized path (using os.path.join / os.path.normpath, or checking a relative suffix) so the test validates the contract without depending on a specific machine layout.
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
Surface the validation and source artifact paths in the read-only pre-trade intake panel so the calculator handoff can be inspected without leaving the UI.
Changes included:
research_uiresearch_uiWhy
The pre-trade intake panel already existed and showed the core decision state.
What was still missing was the artifact path visibility that makes the panel actually useful during operator review. This slice adds that visibility without changing validation, policy, or execution behavior.
Scope
This PR does not:
It only improves read-only intake visibility.
Validation
Validated with:
python -m pytest -q test/test_research_ui_server.py test/test_app_cli.pynode --check research_ui/app.jsgit diff --checkNotes
Closes #185
Summary by Sourcery
Surface pre-trade validation and source artifact information in the read-only research UI intake panel.
New Features:
Tests: