feat: Chart Correlation and Price Relative Strength - #559
Conversation
Both indicators compare a security against a market benchmark, and #498 recorded them as blocked on an API-shape question: the single-series endpoint was thought unable to express "indicator vs. benchmark", needing a new second-symbol parameter first. That premise does not hold. BETA has served exactly this shape since it was added — it fetches SPY as a second series and publishes an ordinary oscillator listing with no benchmark parameter. Verified against production, where BETA returns 1.20–1.78 rather than a degenerate constant, so the benchmark series is real there. Correlation and PRS take the same `(evaluated, benchmark, …)` argument order as ToBeta, so both follow the existing pattern with no API redesign. The benchmark fetch would have been its third copy, so it moves into a `GetVsBenchmark` helper alongside the existing single-series `Get`, and BETA now routes through it. That is the whole of the BETA change: same route, same parameters, same response shape. Correlation charts `correlation` and `rSquared` together — both dimensionless, so one y-axis reads honestly — with a dashed zero line, the point where the security stops tracking the benchmark. PRS charts only `prs`; `prsPercent` is a percentage and would need its own axis, the same mixed-unit problem that split SMA analysis into per-metric listings. PRS gets no threshold line: it is an unnormalized price ratio, so no fixed value marks equal performance, and an invented reference would misinform. The tests pin the property that no status code reveals. When the benchmark resolves to the same bars as the evaluated security the maths still succeeds, returning a constant 1.0 — so a case asserts a divergent benchmark does not correlate perfectly, and a theory asserts all three endpoints request the benchmark by symbol. Confirmed load-bearing: pointing the helper back at its own series fails all four.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: facioquo/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe change adds shared SPY benchmark handling for BETA, CORRELATION, and PRS. It introduces CORRELATION and PRS endpoints and registers both indicators in the catalog. The catalog defines their chart types, parameters, thresholds, and results. Tests verify separate benchmark requests, successful responses, and non-perfect correlation with divergent benchmark data. Assessment against linked issues
Merge Risk: ⚪ Minimal · up to This PR adds chart catalog entries and endpoints for Correlation and PRS while preserving existing benchmark behavior; no actionable merge-blocking risk remains after normal checks and review. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Two defects from self-review of the previous commit. The Correlation listing was inserted before ConnorsRSI, but the catalog is ordered alphabetically by Name and the client renders listings in catalog order, so the indicator picker showed "Correlation Coefficient" ahead of "ConnorsRSI". Moved after it. The new tests were inserted between GenerateSampleQuotes' XML summary and the method itself, leaving the doc attached to the first test instead. Moved back onto the helper.
|
|
FaciōQuōqa Mage Reviewer is standing by.
1 participant
Closes #498.
Summary
The last two chart-less indicator pages —
CorrelationandPrs— now have catalog entries and endpoints. That leavesRenkoas the only remaining item, and it has a proposed "not charted" decision recorded below.The blocker was a false premise
#498 recorded these as blocked on API shape:
That is not the case.
BETAhas served exactly this shape since it was added — it fetches SPY as a second series and publishes an ordinaryoscillatorlisting with no benchmark parameter. And the library signatures line up exactly:ToBeta(sourceEval, sourceMrkt, lookbackPeriods, type)ToCorrelation(sourceA, sourceB, lookbackPeriods)ToPrs(sourceEval, sourceBase[, lookbackPeriods])Same argument order, benchmark second. So both follow the existing pattern with no API redesign, and #474 (OpenAPI spec) is unblocked — it was parked behind this question.
Charting decisions
Correlation charts
correlationandrSquaredon one pane. Both are dimensionless (−1..1 and 0..1), so a shared y-axis does not mislead — unlike SMA analysis, which was split per-metric precisely because its units differed. A dashed line at zero marks where the security stops tracking the benchmark.PRS charts
prsonly.prsPercentis a percentage and would need its own axis — the same mixed-unit problem — so it is omitted rather than crammed in. SincelookbackPeriodsonly drivesprsPercent, the endpoint takes no parameters at all rather than exposing a knob with no visible effect. No threshold line: PRS is an unnormalized price ratio, so no fixed value marks equal performance, and inventing a reference line would misinform.Refactor at the point of contact
The benchmark fetch was about to become its third copy, so it moved into a
GetVsBenchmarkhelper beside the existing single-seriesGet, andBETAnow routes through it. Same route, same parameters, same response shape — verified below.A finding this surfaced (not fixed here)
QuoteBackup.BackupQuotesis a single symbol-agnostic dataset. When storage is unavailable,LoadQuotesAsyncreturns the same bars for bothQQQandSPY, so every benchmark indicator degenerates — correlation becomes exactly1.0at every point. This is pre-existing and already affectsBETA; Correlation just makes it obvious, because a flat 1.0 line reads as broken.Production is unaffected — R2 carries both symbols, confirmed by the live
BETAresponse below returning 1.20–1.78. Reporting rather than fixing, since the failover dataset is its own concern.Test plan
dotnet build Charts.sln -warnAsError— 0 warnings, 0 errorsAssert.IsAssignableFromin my new test; switched toAssert.IsType(…, exactMatch: false))dotnet format --verify-no-changes— cleanCORRELATIONvalues stay within [−1, 1]dataNames matching the JSON payloadBETAregression: unchanged response keys and values after the refactorExecution output
Renko — recording the "not charted" decision
Adopting the proposal already made on #498, which the issue body explicitly sanctions: Renko bricks are not 1:1 with quotes, and the chart architecture windows every indicator dataset by quote index, so a brick series cannot be windowed without a per-indicator axis model. Charting it needs its own pane type with an independent x-axis, which is out of scope for the catalog shape. This flips if a pane type with an independent axis is ever built for another reason.
Notes
<StockIndicatorChart indicator="…" />per page, in the docs repository, plus an indy-charts release carrying nothing new — both catalog entries use existing chart types.SPYand fixed:QuoteServiceserves only the symbols the scheduled refresh maintains, so a caller-supplied symbol would have no data behind it.