Skip to content

Fix RSI producing NaN instead of neutral 50 on flat market - #455

Open
cinar wants to merge 1 commit into
masterfrom
worktree-agent-a09efb63f9fa70505
Open

Fix RSI producing NaN instead of neutral 50 on flat market#455
cinar wants to merge 1 commit into
masterfrom
worktree-agent-a09efb63f9fa70505

Conversation

@cinar

@cinar cinar commented Sep 1, 2026

Copy link
Copy Markdown
Owner

Summary

  • Rsi.ComputeWithContext computed RS = averageGain / averageLoss and then RSI = 100 - (100 / (1 + RS)). When a window has zero gains and zero losses (a flat price run β€” e.g. an untraded bar, or synthetic/test data), averageGain and averageLoss are both 0, so RS = 0/0 = NaN, and the resulting RSI for that bar was NaN instead of the conventional neutral value of 50.
  • Replaced the DivideWithContext β†’ IncrementBy/MultiplyBy/Pow pipeline with a single helper.OperateWithContext that zips averageGains and averageLosses pairwise and special-cases the 0/0 case to 50; the non-flat case computes the identical RS/RSI formula as before, so normal output is unchanged.
  • Checked momentum/connors_rsi.go and other callers of momentum.Rsi/NewRsi β€” they only consume Rsi.ComputeWithContext's output channel and don't depend on its internal pipeline shape, so behavior for non-flat windows is unaffected. Streak-based RSI (used by ConnorsRsi.StreakRsi) shares the same Rsi type and therefore benefits from the same fix, since a streak can also be flat (i.e. 0) with no separate flat-window handling of its own.

Test plan

  • Added TestRsiFlatMarket in momentum/rsi_test.go: a constant closing-price series long enough to produce RSI output, asserting every emitted value is exactly 50 (not NaN).
  • Verified the existing CSV-fixture-based TestRsi is unaffected β€” momentum/testdata/rsi.csv has no flat windows, and the test still passes.
  • go build ./...
  • go vet ./...
  • gofmt -l . (clean for the changed files; unrelated pre-existing formatting diffs exist elsewhere in the repo, outside this change's scope)
  • go test ./... (all packages pass)

πŸ€– Generated with Claude Code

RSI's `ComputeWithContext` computed RS = averageGain / averageLoss and
then RSI = 100 - (100 / (1 + RS)). When a window has zero gains and
zero losses (a flat price run, e.g. an untraded bar or synthetic/test
data), RS was 0/0 = NaN, so the output RSI was NaN for that bar
instead of the conventional neutral value of 50.

Replaced the Divide/IncrementBy/MultiplyBy/Pow pipeline with a single
helper.OperateWithContext that zips averageGains and averageLosses and
special-cases the 0/0 case to 50, otherwise computing the same RS and
RSI formula as before.

Added TestRsiFlatMarket covering a constant closing-price series and
verified the existing CSV-fixture-based TestRsi is unaffected (its
fixture has no flat windows).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xv4stuAb6WuQ8rPZ4cupLp
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 91.69%. Comparing base (f9f68bf) to head (5d00e82).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #455      +/-   ##
==========================================
- Coverage   92.48%   91.69%   -0.79%     
==========================================
  Files         229      230       +1     
  Lines        7302     7355      +53     
==========================================
- Hits         6753     6744       -9     
- Misses        463      525      +62     
  Partials       86       86              

β˜” View full report in Codecov by Harness.
πŸ“’ Have feedback on the report? Share it here.

πŸš€ New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants