Add zero-denominator guards to 8 trend indicators - #502
Merged
Conversation
…ic/Stochastic Fixes NaN/Inf propagation when these trend indicators divide by a quantity that can legitimately be zero on real market data (a flat bar's high==low, a perfectly flat lookback window, an untraded window's zero volume). Each fallback follows the indicator's own established convention rather than an arbitrary value: 0-100 range oscillators (Kdj's RSV/K/D/J, SlowStochastic, Stochastic) fall back to the neutral midpoint 50, matching the existing RSI flat-market fix; signed/zero-centered indicators (Bop, Kama's Efficiency Ratio, Tsi, Cfo) fall back to 0; Vwma falls back to 0 with a documented limitation around forward-filling. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xv4stuAb6WuQ8rPZ4cupLp
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #502 +/- ##
==========================================
+ Coverage 91.96% 92.01% +0.04%
==========================================
Files 232 232
Lines 7653 7661 +8
==========================================
+ Hits 7038 7049 +11
+ Misses 527 524 -3
Partials 88 88 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
Follow-up to #496 ("Tighten high-risk indicators to helper.Float"), which fixed the integer-truncation type-safety gap for these types but deliberately left their live float64 zero-denominator exposure unaddressed. This PR adds that guard to the 8
trend/types from #496's follow-up list, each with a per-indicator fallback decided from its own documented formula/scale rather than an arbitrary value.(Close-Open)/(High-Low)): fallback 0. A zero-range bar (High == Low) forcesOpen == Close == High == Lowtoo, so the numerator is also 0 in that case — 0 is the mathematically correct limit, not a guess, and matches BOP's signed, zero-centered scale (0 = equilibrium).Direction / Volatility): fallback 0. A perfectly flat window makesVolatility(moving sum ofAbs(change)) zero, which also forcesDirectionto zero — ER = 0 ("no efficient movement occurred") matches most published KAMA implementations' explicit zero-volatility case.RSV = (Close-Min)/(Max-Min)*100, thenK/D/Jderived from RSV): fallback 50. RSV is a 0-100 range ratio identical in shape to Stochastic's %K, so a zero range is treated as the neutral midpoint of that scale, same convention as RSI's existing flat-market fix (Fix RSI producing NaN instead of neutral 50 on flat market #455).TSI = (PCDS/APCDS)*100): fallback 0.APCDS(smoothed absolute price change) is zero only when price has been perfectly flat — no momentum to report, which is TSI's neutral center on its signed -100..100 scale.Sum(Price*Volume)/Sum(Volume)): fallback 0, documented as a known limitation. A window with zero volume (no trading) has no real volume-weighted price to report. Forward-filling the last valid average would arguably be more representative for plotting purposes, but requires per-window state that no other guard in this indicator family needs; 0 keeps the fix consistent with the other 7 stateless per-bar guards, and the limitation is called out explicitly in the doc comment.((Price-Forecast)/Price)*100): fallback 0. Lowest-priority/weakest real-world risk (only triggers on a degenerate zero-price input) — 0 means "no forecast deviation to report."Each guard is a plain
if denom == 0 { return fallback }check immediately before the division, matching the existing pattern inmomentum/ibs.goandvolatility/chop.go. Doc comments on each type explain the chosen fallback and why.Out of scope (per the task): momentum/volatility/volume package guards are separate parallel work;
examples/,strategy/, README, and legal docs are untouched.Test plan
TestBopFlatBar,TestKamaFlatMarket,TestKdjFlatMarket,TestTsiFlatMarket,TestVwmaNoVolume,TestCfoZeroPrice,TestSlowStochasticFlatMarket,TestStochasticFlatMarket), each asserting the documented fallback value instead of NaN/Inf/panic.TestKdjFlatMarket/TestSlowStochasticFlatMarket/TestStochasticFlatMarketI hit a real deadlock: these types fan out K/D(/J) internally through unbufferedDuplicateWithContext, which requires every output channel to be drained concurrently — fully draining one viahelper.ChanToSlicebefore touching the others stalls the whole pipeline. Fixed by collecting each output channel on its own goroutine (sync.WaitGroup), matching the concurrent-read pattern the existingTestKdj/TestStochasticfixture tests already use viahelper.CheckEquals.TestBop,TestKama,TestKdj,TestTsi,TestVwma,TestCfo,TestSlowStochastic,TestStochastic) are unaffected — none of the fixtures contain a flat/zero-range bar, and all still pass byte-for-byte against their expected output.go build ./...go vet ./...gofmt -l .clean on all touched files (pre-existing unrelated gofmt drift elsewhere in the repo, outside this change's scope)go test ./...— all packages passhttps://claude.ai/code/session_01Xv4stuAb6WuQ8rPZ4cupLp