fix(ta): return na, not an invented constant, on three degenerate arms#137
Merged
Conversation
ta.tsi, ta.cog and ta.cmo each guard a zero-denominator case by returning a
hardcoded number (0.0) that is not derivable from the published Pine formula.
The values were author's convenience, not derivations -- the sibling Stoch arm
still carries the original comment ("Avoid division by zero; midpoint when
flat") that shows how they were chosen. ta.wpr, whose degenerate condition is
structurally identical, already returned na, so the engine was also internally
inconsistent with itself.
A clean-room TradingView oracle was exported and independently adjudicated on
2026-07-25 to settle what TradingView actually returns. It licenses exactly
these three arms and no others:
ta.tsi -> na the denominator is an EMA of |change|, which reaches exactly
0.0 only when the source is flat from bar 0, so the oracle's
all-constant construction is not a special case -- it is the
only reachable one.
ta.cog -> na a zero window sum is structurally unreachable for positive
prices, so the zero-sum construction likewise is the case.
ta.cmo -> na TradingView does not guard this arm at all; it evaluates the
formula, so 0/0 surfaces as na.
The governing principle, which is what bounds this change: the engine's guard
fires exactly when *the engine's own* denominator is bitwise 0.0, so the
on-point oracle is one where *TradingView's* denominator is also bitwise 0.0.
The all-constant construction guarantees that for these three arms.
Deliberately NOT changed, because the same oracle refutes the obvious
uniform fix for them:
ta.stoch (50.0) known-wrong but the correct value is UNRESOLVED. na is wrong
too: on a flat window of a varying source TradingView
returned a number that is neither na nor 50. The only
mechanism consistent with both probe families is
carry-forward, which is inferred, not observed -- and
shipping an inferred mechanism is what got a previous fix
reverted.
ta.cci (0.0) SPLIT. Exactly 0.0 on an all-constant source but measurably
not 0 at a flat window of a varying source. The two surviving
hypotheses demand opposite fixes, so na would be wrong under
both.
ta.mfi (100.0) UNVERIFIED. Agrees in the all-constant construction but is
untested at a real flat window, and ta.cci is the standing
proof that this construction can mislead.
Tests, written RED first and covering compute() and recompute() for each arm
(COG::recompute holds an independent copy of the guard; CMO/TSI delegate):
- test_ta_osc_edge.cpp gains one degenerate case per fixed arm, each with a
non-vacuous control so the assertion cannot pass by warmup, plus a NEGATIVE
control pinning stoch/cci/mfi at their current constants so a well-meaning
edit that "finishes the job" fails loudly.
- test_ta_osc_edge.cpp's header claimed the expected values were
"Pine-correct, derived by hand from the documented behaviour". The oracle
falsifies that for four of the six arms; the claim is retracted and
replaced with the per-arm evidence status.
- test_ta_indicators_extras.cpp asserted the three old constants directly
(test_cmo / test_tsi / test_cog). Those assertions were falsified by the
oracle and are inverted here.
Honest limit on impact: a prior audit measured every one of these arms as
UNREACHABLE on the committed feed (0 bars with high == low; longest runs of
identical values 2-4 against minimum lengths in use of 5-25), and TSI/COG are
unreachable structurally at any timeframe. This is a spec-correctness fix with
no expected measurable effect, and the corpus gate confirms it: all 254
engine_trades.csv tapes and validation_report.md are byte-for-byte identical
before and after (252 ran, ok=252, excellent=247, strong=4, anomaly=1).
ctest 121/121, unchanged from baseline.
Not addressed here: ta.correlation carries the same defect shape
(src/ta_misc.cpp, den == 0 -> 0.0, in both compute and recompute). The oracle
carried no correlation leg, so it is out of scope and remains open.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR aligns three TA oscillators (ta.cmo, ta.tsi, ta.cog) with Pine/TradingView behavior at exact zero-denominator singularities by returning na (IEEE NaN) instead of previously hardcoded constants, and updates tests to pin the corrected behavior while explicitly preserving known-uncertain “degenerate constant” arms.
Changes:
- Update
CMO,TSI, andCOGto returnna<double>()when their denominators are exactly zero (includingCOG::recompute). - Expand edge-focused oscillator tests to cover compute + recompute behavior for the three fixed degenerate arms, and add a negative-control test that intentionally pins
stoch/cci/mficurrent constants. - Update indicator extra tests to expect
nafor the corrected degenerate cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/ta_oscillators.cpp |
Switch CMO/TSI/COG (compute + COG recompute) zero-denominator behavior from invented constants to na. |
tests/test_ta_osc_edge.cpp |
Add dedicated degenerate-arm coverage for CMO/TSI/COG and a negative-control pin for unresolved arms. |
tests/test_ta_indicators_extras.cpp |
Update existing oscillator assertions to match na behavior for the three corrected degenerate branches. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
270
to
+272
| ta::CMO flat(3); | ||
| for (int i = 0; i < 4; ++i) flat.compute(5.0); | ||
| CHECK(near(flat.compute(5.0), 0.0)); | ||
| CHECK(is_na(flat.compute(5.0))); |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The bug
Three oscillators returned an invented constant where the published formula's
denominator is exactly zero, instead of propagating
na:ta.cmo0.0ta.tsi0.0ta.cog0.0These are author's-convenience values, not derivations — the sibling
ta.wpralready returns
naon the structurally identicalhh == ll. At the degeneratepoint numerator and denominator are both exactly 0, and Pine surfaces IEEE NaN as
na.Why only these three
A clean-room TradingView probe was exported and independently adjudicated. It ran
21 legs on a canonical chart: per oscillator, one leg gated on
not na(...)and onegated on equality with the engine's constant, plus a liveness control, a
positive control, and two forced-
nacontrols.The probe contradicted the obvious inference. "Pine yields
nafor 0/0, so allsix invented constants are defects" is false for two of them: TradingView
returned exactly
0.0forta.cciand exactly100.0forta.mfi(both legsfiring 21/21). Shipping a uniform
nachange would have broken two arms that arenot defective in that construction.
So this PR fixes only the three arms where TradingView demonstrably returns
na(
CMO_NUM,TSI_NUM,COG_NUMeach 0/21) and deliberately leaves untouched:ta.stoch— its50.0is indefensible, butnais wrong too: on a flat window ofa varying source TradingView returned a number that is neither. The only
mechanism consistent with both probe families is carry-forward, which is inferred,
not measured. Not shipped.
ta.cci— split result; the two surviving hypotheses demand opposite fixes.ta.mfi— agrees in one construction but untested at a real flat window;ta.cciis the standing proof that construction can mislead.Probe self-test passed: the liveness controls fired and both forced-
nacontrolsstayed silent, independently confirming that plain Pine
0.0 / 0.0isna.Falsified test evidence, corrected
Four pieces of test evidence asserted the old behaviour and are corrected here:
tests/test_ta_indicators_extras.cpp:268/:293/:340each required theinvented
0.0for CMO / TSI / COG. Inverted tois_na, each citing the probe.tests/test_ta_osc_edge.cpp:23-24claimed these constants were "Pine-correct,derived by hand from the documented behaviour". That claim is false for these
arms; it is replaced with a per-arm evidence status block.
A negative control now pins
ta.stoch50.0,ta.cci0.0andta.mfi100.0(compute and recompute), so a future change that "finishes the job" on anunlicensed arm fails loudly. It passes both before and after this change — it is
anchored to current behaviour, not to this fix.
Gates
of the three arms. Each fixed arm also carries a positive control asserting a finite
non-degenerate value, so an
is_naassertion cannot pass merely from warmup.with all 252/252 tapes byte-identical.
base.
Honest limits
These arms are unreachable on the current validation feed — no bar has
high == low, and the longest identical-value runs (2-4 bars) fall short of everylength in use (5-25). So this is a spec-correctness fix with no expected
measurable effect; the byte-identical artifacts are the acceptance criterion, not
an absence of testing. A single changed tape would have falsified that analysis.
For
ta.cmothenais correct wherever the engine's own guard fires; ifTradingView's sums drift at a real flat run, engine and TV could still diverge there.
That is a separate numerics question, not made worse by this change.
A fourth arm in the same family,
ta.correlation(src/ta_misc.cpp:174compute,:294recompute,den == 0 -> 0.0), is left untouched because the probe carried noleg for it. It should not be changed on the strength of the pattern — this family
demonstrably does not behave uniformly.