Create skill.json#6
Conversation
WalkthroughA new Changesagentfx Skill Definition
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment Warning |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skills/agentfx/skill.json`:
- Around line 45-56: Add an explicit market identifier to the start_bot_loop
request schema so the backend can trade a specific asset deterministically.
Update the request_body in skill.json to require a mint (or equivalent market
identifier) alongside wallet and strategy, and make sure the schema aligns with
the inputs expected by get_market_data and create_bot_quote. Verify any callers
of start_bot_loop are updated to pass the new field consistently.
- Around line 60-63: The stop_bot_loop endpoint is missing a selector to
identify which running loop to stop, while the start flow already scopes by
wallet; update the skill definition in stop_bot_loop to include the same loop
identity (for example wallet or another unique loop key) in its parameters or
request body so the backend can target the correct bot instance. Use the
existing stop_bot_loop and start-loop contract entries in skill.json as the
place to align the request shape and make the stop action unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 67b1bb8f-e5b3-4005-b6ca-b1705903763a
📒 Files selected for processing (1)
skills/agentfx/skill.json
| "request_body": { | ||
| "wallet": { "type": "string", "required": true }, | ||
| "strategy": { | ||
| "type": "object", | ||
| "properties": { | ||
| "entryThreshold": { "type": "number", "default": 0.05 }, | ||
| "exitThreshold": { "type": "number", "default": 0.1 }, | ||
| "maxPositionUsd": { "type": "number", "default": 100 }, | ||
| "takeProfitPct": { "type": "number", "default": 0.2 }, | ||
| "stopLossPct": { "type": "number", "default": 0.1 } | ||
| } | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Add an explicit market identifier to start_bot_loop.
This request schema never tells the backend what asset the loop should trade. get_market_data and create_bot_quote both require mint, but Line 45-56 only send wallet and strategy, so callers cannot start a deterministic loop for a specific market.
Proposed fix
"request_body": {
"wallet": { "type": "string", "required": true },
+ "mint": { "type": "string", "required": true },
"strategy": {
"type": "object",
"properties": {📝 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.
| "request_body": { | |
| "wallet": { "type": "string", "required": true }, | |
| "strategy": { | |
| "type": "object", | |
| "properties": { | |
| "entryThreshold": { "type": "number", "default": 0.05 }, | |
| "exitThreshold": { "type": "number", "default": 0.1 }, | |
| "maxPositionUsd": { "type": "number", "default": 100 }, | |
| "takeProfitPct": { "type": "number", "default": 0.2 }, | |
| "stopLossPct": { "type": "number", "default": 0.1 } | |
| } | |
| } | |
| "request_body": { | |
| "wallet": { "type": "string", "required": true }, | |
| "mint": { "type": "string", "required": true }, | |
| "strategy": { | |
| "type": "object", | |
| "properties": { | |
| "entryThreshold": { "type": "number", "default": 0.05 }, | |
| "exitThreshold": { "type": "number", "default": 0.1 }, | |
| "maxPositionUsd": { "type": "number", "default": 100 }, | |
| "takeProfitPct": { "type": "number", "default": 0.2 }, | |
| "stopLossPct": { "type": "number", "default": 0.1 } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/agentfx/skill.json` around lines 45 - 56, Add an explicit market
identifier to the start_bot_loop request schema so the backend can trade a
specific asset deterministically. Update the request_body in skill.json to
require a mint (or equivalent market identifier) alongside wallet and strategy,
and make sure the schema aligns with the inputs expected by get_market_data and
create_bot_quote. Verify any callers of start_bot_loop are updated to pass the
new field consistently.
| "name": "stop_bot_loop", | ||
| "description": "Triggers immediate teardown of the active on-chain trading loop.", | ||
| "path": "/bot/stop", | ||
| "method": "POST" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
stop_bot_loop needs a loop selector.
Line 60-63 define a stop action with no parameters or body, while Line 46 requires a wallet to start one. Unless the backend guarantees a single global loop, this contract cannot unambiguously target the running bot.
Proposed fix
{
"name": "stop_bot_loop",
"description": "Triggers immediate teardown of the active on-chain trading loop.",
"path": "/bot/stop",
- "method": "POST"
+ "method": "POST",
+ "request_body": {
+ "wallet": { "type": "string", "required": true }
+ }
}📝 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.
| "name": "stop_bot_loop", | |
| "description": "Triggers immediate teardown of the active on-chain trading loop.", | |
| "path": "/bot/stop", | |
| "method": "POST" | |
| "name": "stop_bot_loop", | |
| "description": "Triggers immediate teardown of the active on-chain trading loop.", | |
| "path": "/bot/stop", | |
| "method": "POST", | |
| "request_body": { | |
| "wallet": { "type": "string", "required": true } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@skills/agentfx/skill.json` around lines 60 - 63, The stop_bot_loop endpoint
is missing a selector to identify which running loop to stop, while the start
flow already scopes by wallet; update the skill definition in stop_bot_loop to
include the same loop identity (for example wallet or another unique loop key)
in its parameters or request body so the backend can target the correct bot
instance. Use the existing stop_bot_loop and start-loop contract entries in
skill.json as the place to align the request shape and make the stop action
unambiguous.
Summary by CodeRabbit
agentfx-magicblock-skill.