Conversation
kbearXD
requested review from
bailantaotao,
c9s,
Copilot,
dboyliao,
gx578007 and
ycdesu
There was a problem hiding this comment.
Pull Request Overview
Introduces the new XVS strategy implementing volume breakout entries and engulfing/pivot-based exits, and registers it in the builtin strategies.
- Adds full strategy implementation (parameter validation, subscriptions, entry/exit logic, indicators).
- Registers the strategy in builtin.go for automatic loading.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pkg/strategy/xvs/strategy.go | Implements the XVS strategy with entry (volume + EMA filter) and exit (pivot high + engulfing) logic. |
| pkg/cmd/strategy/builtin.go | Registers the new XVS strategy for inclusion in the strategy set. |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return | ||
| } | ||
|
|
||
| if kline.GetClose().Compare(kline.GetOpen()) < 0 { |
There was a problem hiding this comment.
The condition assigns lastGreenKline when the candle is red (close < open), contradicting the intent to store a green candle for later engulfing exit checks. Change the comparison to > 0 so only green candles are stored: if kline.GetClose().Compare(kline.GetOpen()) > 0 { s.lastGreenKline = kline }.
| if kline.GetClose().Compare(kline.GetOpen()) < 0 { | |
| if kline.GetClose().Compare(kline.GetOpen()) > 0 { |
| bbgo.Notify("Found %s volume breakout: volume %s, quote volume %s, price %s", | ||
| s.Symbol, | ||
| kline.Volume.String(), | ||
| kline.QuoteVolume.String(), | ||
| closePrice.String(), | ||
| kline) |
There was a problem hiding this comment.
The format string has four %s verbs but five arguments are passed; the extra kline argument will produce a %!(EXTRA ...) formatting artifact. Remove the unused kline parameter or add a %s (or %v) verb for it (e.g. append ", kline %v" to the format string and keep the argument).
| bbgo.Notify("Submitting %s market buy order with quantity %s", | ||
| s.Symbol, | ||
| s.Quantity.String(), | ||
| orderForm) |
There was a problem hiding this comment.
The notification always says "market buy order" even when orderForm.Type was switched to limit, and it logs s.Quantity instead of the actual quantity variable (which may come from scaling). Use orderForm.Type and orderForm.Quantity.String() to ensure accurate reporting, and update the format string accordingly.
| bbgo.Notify("Submitting %s market buy order with quantity %s", | |
| s.Symbol, | |
| s.Quantity.String(), | |
| orderForm) | |
| bbgo.Notify("Submitting %s %s buy order with quantity %s", | |
| s.Symbol, | |
| orderForm.Type, | |
| orderForm.Quantity.String()) |
| // Validate validates the validity of strategy parameters | ||
| func (s *Strategy) Validate() error { | ||
| if s.Quantity.IsZero() && s.ScaleQuantity == nil { | ||
| return fmt.Errorf("quantity or scaleQuantity can not be zero") |
There was a problem hiding this comment.
Corrected wording from "can not" to "cannot".
| return fmt.Errorf("quantity or scaleQuantity can not be zero") | |
| return fmt.Errorf("quantity or scaleQuantity cannot be zero") |
c9s
force-pushed
the
kbearXD/xvs/strategy
branch
from
9214646 to
02bdc45
Compare
No description provided.