[Bug] Strat cache hit drops hyperparams

[axlnur]

Bug描述 Describe the bug

strat.New caches the constructed *TradeStrat and returns early on a cache hit:

cacheKey := utils.MD5([]byte(polID + "\n" + pol.ToYaml()))
cacheMu.Lock()
defer cacheMu.Unlock()
if obj, ok := cacheStrats[cacheKey]; ok {
    return obj                      // <- factory is skipped
}
makeFn, ok := StratMake[pol.Name]
stgy = makeFn(pol)                  // <- this is what registers the hyper-params

The strategy factory is the only thing that registers hyper-parameter definitions — it is
where pol.Def(...) / pol.DefInt(...) run. On a cache hit the factory does not run, so the
caller's *RunPolicyConfig is left with an empty defs map, even though the returned
*TradeStrat is correct.

optForPol reads the hyper-params from its own policy object immediately afterwards
(opt/hyper_opt.go):

_ = strat.New(pol)
params := pol.HyperParams()
if len(params) == 0 {
    log.Warn("no hyper params, skip optimize", zap.String("strat", title))
    return 0, nil
}

So the policy is silently skipped: no error, exit code 0, and the optimize run "succeeds"
having optimized nothing. The only trace is one log.Warn that reads like a configuration
notice rather than a failure.

The most damaging path is rolling optimization (bt-opt). rollBtOpt.next() reuses the same
policy objects for every window:

config.RunPolicy = t.initPols          // the SAME []*RunPolicyConfig, every window
polStr, err = runOptimize(t.args, 0)   // -> optAndPrint(gp.Clone(), ...)

RunPolicyConfig.Clone() is a shallow struct copy (res := *c), and defs is unexported and
lazily created, so each window's clone starts with no defs of its own. Window 1 populates its
clone and caches the strategy under that identity; windows 2..N hit the cache, keep an empty
defs map, and are skipped. Only the first window is actually optimized.

具体复现流程？Steps To Reproduce

Hermetic — no database, no exchange, no strategy files. Drop into strat/:

package strat

import (
	"testing"

	"github.com/banbox/banbot/config"
	"github.com/banbox/banbot/core"
)

func TestBtOptWindowsLoseHyperParams(t *testing.T) {
	calls := 0
	StratMake["probe"] = func(pol *config.RunPolicyConfig) *TradeStrat {
		calls++
		pol.Def("zEntry", 1.5, core.PNorm(0.8, 3.0))
		pol.Def("atrMult", 2.5, core.PNorm(1.0, 5.0))
		return &TradeStrat{OnBar: func(s *StratJob) {}}
	}
	initPol := &config.RunPolicyConfig{Name: "probe", RunTimeframes: []string{"1h"}}

	for win := 1; win <= 3; win++ {
		gp := initPol.Clone() // exactly what runOptimize does per window
		New(gp)
		t.Logf("window %d: hyper params = %d", win, len(gp.HyperParams()))
	}
	t.Logf("strategy factory ran %d times (out of 3 windows)", calls)
}

Observed on main @ a132a3b:

window 1: hyper params = 2
window 2: hyper params = 0        <- optForPol would skip it
window 3: hyper params = 0        <- optForPol would skip it
strategy factory ran 1 times (out of 3 windows)

End to end: run bt-opt with any strategy that declares hyper-params via pol.Def /
pol.DefInt and a review/run period that yields several windows. Only the first window's
opt_*.log contains real trials; the rest log no hyper params, skip optimize.

期望行为 Expected behavior

Every window optimizes. A cache hit should not change what the caller's policy knows about
itself — strat.New(pol) should leave pol.HyperParams() populated whether or not the
strategy came from the cache.

截图 Screenshots

None. The test output above is the whole signal; the failure is silent otherwise.

运行环境 Running Environment

• OS: MacOS
• CPU: arm
• Version: banbox/banbot@main HEAD a132a3b (2026-07-28) / v0.4.2-beta.10

其他信息 Additional context

Suggested fix — propagate the definitions on the hit, keeping the cached object as-is:

if obj, ok := cacheStrats[cacheKey]; ok {
    // A hit means the factory already registered this policy's hyper-params on the object
    // it was constructed from; the caller's pol is a different instance whose defs are
    // still empty, and optForPol reads them from ITS pol.
    if obj.Policy != nil && obj.Policy != pol {
        pol.MergeDefsFrom(obj.Policy)
    }
    return obj
}

with a small helper on RunPolicyConfig that fills only the gaps (an existing def always
wins), since defs is unexported:

func (c *RunPolicyConfig) MergeDefsFrom(o *RunPolicyConfig) {
	if o == nil || len(o.defs) == 0 {
		return
	}
	if c.defs == nil {
		c.defs = make(map[string]*core.Param, len(o.defs))
	}
	for k, v := range o.defs {
		if _, ok := c.defs[k]; !ok {
			c.defs[k] = v
		}
	}
}

We have been running this exact patch in a fork since mid-July with no side effects.

Two adjacent hardening ideas, lower priority:

1. optForPol's len(params) == 0 branch is indistinguishable between "this strategy has no
   tunable parameters" (legitimate) and "we failed to obtain them" (this bug). Distinguishing
   the two — or at least raising the log level when the strategy is known to declare
   parameters — would have surfaced this immediately.
2. Nothing ever clears cacheStrats, so it also grows for the life of the process and keeps
   strategy closures (with their baked parameters) alive across optimize trials. A reset at
   the per-trial boundary — resetOptimizeTrialVars is the natural place — makes trial
   isolation explicit rather than relying on the cache key covering every construction input.

微信/QQ/Telegram/Discord/Email

• alx@nur.re