You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The PR appears safe to merge on the findings assessed here, subject to the stated backend-PR dependency.
Summary
The PR adds Phoenix state presets, market discovery and scenario creation, editor support, and a constrained market dropdown. The changes since the previous review consolidate field extraction and MCP parsing and adjust the shared Listbox structure.
P1 — Phoenix workflows call nonexistent backend routes (scenarios-api.ts:69): The coupled Surfpool router does not register phoenix-markets, phoenix-collateral, phoenix-direct-mark, or phoenix-reference-prices. All newly exposed Phoenix flows therefore fail with 404s. Add the backend routes before exposing these controls, or use existing supported endpoints.
P2 — Listboxes no longer avoid viewport boundaries (listbox.tsx:83): Replacing Headless UI’s anchored positioning with absolute top-full removes automatic flipping and available-height calculation. Listboxes near the bottom of a dialog or viewport can render partially off-screen. Retain anchored positioning and constrain its calculated available height.
High:scenarios-api.ts:68 calls four new Phoenix endpoints, but this PR provides no corresponding server routes. Market discovery and all creation flows will return 404 unless the backend changes land first. Add/stack the required server implementation or gate the UI until available.
High:scenarios-api.ts:68 calls four new Phoenix endpoints, but this PR provides no corresponding server routes. Market discovery and all creation flows will return 404 unless the backend changes land first. Add/stack the required server implementation or gate the UI until available.
could you please check this PR here LimeChain/surfpool#9 as i believe it resolves the above mentioned reported problem
P1 – Invalid Phoenix markets can be saved as scenarios.phoenix-state-dialog.tsx:68 accepts any non-empty symbol, while scenarios-api.ts:135 posts directly to the generic scenario endpoint. This bypasses live market discovery/validation, so typos like BTCC can create unusable scenarios that fail only when applied. Restore market discovery or validate through the Phoenix tool/backend before creation.
scenarios-api.ts:237: new URL(payload.url) rejects relative scenario URLs such as /scenarios?id=..., although these are valid MCP responses. Resolve against studioUrl, e.g. new URL(payload.url, studioUrl), and add a relative-URL test.
scenarios-bento.types.ts:95: The cascade prompt names a trader and market but instructs create_scenario values containing only quote_lot_collateral and target_ticks. Explicitly include the trader and symbol bindings in their respective overrides; otherwise the generated scenario may target unresolved/default accounts.
phoenix-state-dialog.tsx:142: Clear symbolOptions before each market lookup. If a previous lookup succeeded and a later one fails, the old catalog remains and incorrectly validates/rejects symbols for the new Studio instance.
[P3] Preserve field spacing around listboxes — listbox.tsx:23: The new wrapper leaves data-slot="control" on the nested button. Catalyst’s Field spacing selectors require the control to be a direct child, so labels, descriptions, and errors lose their margins when used with this component. Add data-slot="control" to the wrapper or remove the unnecessary wrapper.
Tests weren’t run because dependencies aren’t installed. Backend compatibility could not be verified from this repository.
[P2] Editing loses the live PerpAssetMap address.scenarios-api.ts:179 creates overrides against the discovered map, but updateActionInSlot replaces their account with action.template.address. When the live map differs from the template, clicking Update Action silently retargets the scenario. Preserve the saved account when editing, and test creation → editing with different live/template addresses.
Tests were not run because dependencies are not installed.
[P2] Newly added editor actions still target the static market account.scenario-editor.tsx:130 fetches symbols but discards the live perpAssetMap. “Add Action” consequently saves template.address. On forks with a migrated map, these actions target the wrong account. Retain the catalog address and use it for new Phoenix market actions.
[P3] Listbox wrapper breaks Field spacing.listbox.tsx:23 moves data-slot="control" beneath an unmarked wrapper, breaking Field’s direct-child spacing selectors. Put data-slot="control" on the wrapper.
Tests were not run; dependencies are unavailable in this checkout.
[P2] Wait for live market discovery before saving new actions — scenario-editor.tsx:135. Catalog loading neither clears previous dynamicOptions nor disables “Add Action.” Saving while discovery is pending can persist a stale PerpAssetMap or the template’s fixed address. Closing the panel then cancels the discovery update, leaving that incorrect address saved. Track loading per Studio URL/source and require successful address resolution before adding a Phoenix market action.
Review limited to the specified changes. Tests were not run.
[P2] Failed market discovery still permits saving a stale account address. In override-account.ts:16, missing catalog data silently falls back to the template address. Since discovery errors return an empty catalog and re-enable Add Action, a new Phoenix action can target the old PerpAssetMap after migration. Require successful address discovery for new dynamic actions and offer retry; saved actions can retain their existing address.
Could not run the focused tests because pnpm is unavailable.
[P2] Prevent saving Phoenix actions without a market. In scenario-editor.tsx:129, market discovery runs independently of account loading, but “Add to Selected Slot” only checks whether a slot and action are selected. Users can save while markets are loading—or after discovery fails—with no symbol, producing an invalid Phoenix override. Disable adding until a required market is selected, and show discovery failures with a retry option. Preserve the ability to edit saved actions whose symbols are already present.
[P2] Preserving saved accounts can break PDA selector edits — scenario-editor.tsx:748. Updating an existing action now retains its saved account, including resolved PDA seeds. Changing a token/feed selector updates values but leaves those seeds pointing to the previous account. Preserve explicit pubkeys for Phoenix, but rebuild property-dependent PDA seeds when selectors change. Add a regression test that changes a saved PDA selector and checks the submitted account.
P2 — Manually added Phoenix market actions can use stale state.scenario-editor.tsx:672 forks the market map when selecting an action, but handleActionSelect defaults fetchBeforeUse to false. Waiting before playback can therefore trigger Phoenix’s mark-staleness validation. Default these actions to fetchBeforeUse: true, as the preset creation flow does.
P2 — Review CI fails for fork PRs.openai-review.yaml:38 requires OPENAI_API_KEY, which GitHub withholds from fork-triggered pull_request workflows. Add a guard to skip unsupported runs cleanly.
Tests were not run; dependencies are not installed.
[P1] Editing PDA selectors can retain the old account.scenario-editor.tsx:836 now preserves the saved account for every unchanged template. For saved PDAs with resolved seeds, changing a token or configuration selector updates the values but retains the previous PDA seeds, potentially targeting the wrong account. Preserve explicit pubkeys where needed, but rebuild PDA addresses from template references. Add a regression test that changes a saved PDA selector.
Tests weren’t run because dependencies aren’t installed.
[P2] Preserve custom markets when reopening the dialog — phoenix-state-dialog.tsx:182. Every catalog reload replaces an unlisted custom symbol with the first listed market, while retaining the entered ticks or margin factor. Closing and reopening can therefore create a shock for a different market. Preserve nonempty custom selections; apply the catalog default only before the user selects a market.
Tests could not be run because dependencies are not installed.
[P2] Preserve custom markets when reopening the dialog — phoenix-state-dialog.tsx:191: Loading the catalog replaces any unlisted symbol with the first listed market. Entering a custom market, cancelling, and reopening silently changes the market while retaining the tick or margin target. Preserve nonempty custom values and add a reopen regression test.
Tests were not run; dependencies are not installed.
Validate Phoenix values in the editor too.scenario-editor.tsx renders market parameters as unrestricted text inputs. Users can save values outside the dialog’s limits (mark ticks: 1–4294967295; risk factor: 1–10000). Reuse the dialog validators and show errors before saving.
Add tests using the real Combobox. The Phoenix dialog and editor tests mock it, leaving Headless UI keyboard selection, custom-value commitment, and dialog portal behavior unverified.
No other definite bugs found in the PR diff. Tests weren’t run because dependencies aren’t installed.
Add Phoenix validation in the editor (scenario-editor.tsx). The new value_type inputs accept arbitrary strings, allowing edits such as target_ticks = 0 or maintenance_risk_factor_bps = 10001, which the creation dialog rejects. Apply the same range checks before updating an action and show inline feedback.
No other concrete issues found in the PR changes. Tests weren’t run; dependencies are unavailable locally.
[P2] Review workflow fails on fork PRs — openai-review.yaml:35: pull_request workflows from forks cannot access OPENAI_API_KEY, so the Codex step fails. Their GITHUB_TOKEN also cannot post comments despite the requested write permissions. Skip fork PRs explicitly, or use a separate trusted workflow to publish feedback.
[P2] Unit fallback silently reinterprets an entered amount — phoenix-state-dialog.tsx:143. If a user enters a USD target, then selects a custom market without conversion metadata—or reopens the dialog and market discovery fails—activeChoice falls back to raw ticks while retaining amount. A $85,000 target becomes 85,000 ticks, and creation remains enabled. Clear the amount or require an explicit unit selection when the selected unit becomes unavailable.
Tests weren’t run because dependencies and pnpm are unavailable in this workspace.
[P2] Silent unit changes can create an unintended price shock — phoenix-state-dialog.tsx:145. Selecting a custom market without metadata falls back from % or USD to ticks while retaining both the amount and the previous unit state. Returning to a listed market silently restores that previous unit: entering 1500 ticks for a custom market can become a +1500% adjustment. Clear the amount and update the unit when availability changes, or require explicit unit selection.
Tests could not be run because dependencies are not installed.
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
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.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Adds Phoenix Eternal scenario support to Studio:
Verification
Pairs with LimeChain/surfpool#9.
The PR appears safe to merge on the findings assessed here, subject to the stated backend-PR dependency.
Summary
The PR adds Phoenix state presets, market discovery and scenario creation, editor support, and a constrained market dropdown. The changes since the previous review consolidate field extraction and MCP parsing and adjust the shared Listbox structure.
Diagram
%%{init: {'theme': 'neutral'}}%% flowchart LR Preset[Phoenix state preset] --> Dialog[State dialog] Dialog --> Market[Live market selection] Dialog --> Collateral[Collateral stress] Market --> Template[Phoenix market template] Template --> API[Scenario API] Collateral --> MCP[Phoenix MCP tool] API --> Editor[Scenario editor] MCP --> EditorReviews (19) · Last reviewed commit: "refactor(studio): trim the Phoenix integ..."