feat(trading-strategies): deterministic momentum "too-late" scorecard (+ FMP data client) - #1136
Open
bennycode wants to merge 6 commits into
Open
feat(trading-strategies): deterministic momentum "too-late" scorecard (+ FMP data client)#1136bennycode wants to merge 6 commits into
bennycode wants to merge 6 commits into
Conversation
…card Scores momentum names against an explicit, reproducible rubric so the same inputs always yield the same ranking — answering "have I already missed the entry?" rather than just "is this a good company?". - computeScorecard: pure points rubric over upside-to-target, extension vs the 200-day moving average, forward P/E, EPS growth, and analyst rating. Deterministic with stable tiebreaks; a stock's score never depends on the others. - MomentumScorecard: thin orchestrator that fills the inputs from FMP. - exchange: new typed FmpAPI client (quote, price-target-consensus, analyst-estimates, ratings-snapshot) with zod looseObject schemas, mirroring AlpacaAPI. Unit tests pin every rubric band, loss-making handling, order-independence, and forward-year selection.
A second rubric fed entirely by TipRanks data, for when FMP's forward estimates aren't available. Keeps the two metrics TipRanks supplies directly (upside to target, extension vs the 200-day MA) and swaps the forward-looking bands — forward P/E and EPS growth — for TipRanks' own signals: Smart Score, analyst consensus, and trailing P/E. Reuses the shared scoreUpside/scoreExtension bands and the same stable tiebreaks, so its ranking is directly comparable to the FMP scorecard. The trade-off is by design: without forward earnings it systematically under-ranks high-forward-growth momentum names (e.g. memory/semis), which the doc comment and tests make explicit.
…FMP scorecard Re-admits "trend" to the fundamental scorecard in a disciplined way: a sixth band scores the recent move in the average analyst price target (last month vs last quarter, via FMP's price-target-summary). Rising targets are the fundamental tell that a move will persist rather than exhaust — the signal that distinguishes a still-running momentum name from an exhausted one, and one that is not captured by valuation, growth, or a composite sentiment score. - exchange: new getPriceTargetSummary on FmpAPI + schema. - A zero last-month target means "no target issued recently" (missing data), not a 100% cut, so it scores neutral instead of getting hammered. Effect: MU rises to the top (analysts still hiking its target offsets its extension), while genuinely overshot names (WDC/STX/INTC) stay down despite rising targets.
Closes the gap ON Semiconductor exposed: after gapping down ~23% it briefly scored *better* (price crashed while the analyst target stayed stale, so upside looked big and extension shrank) and ranked up. The guard demotes a name that has dropped well through its 50-day MA, where the target is stale and the upside is fiction until estimates catch up. - Threshold-based (>8% below the 50-day) so an ordinary 2-3% dip doesn't trip it — only a real break does. GOOGL's mild pullback is spared; ON's 17% break is caught. - Exposes `trendBroken` on each row for transparency. Effect: ON drops from the top tier to mid-pack; GOOG/GOOGL keep their spots.
The first guard only neutralized a crash: a flat -3 that the crash-inflated upside and extension bands cancelled out, so ON Semiconductor still sat at #6 after a 23% gap-down. Two changes fix the root cause: 1. When a trend is broken, stop trusting the stale-target bands. Cap upside and extension at zero instead of scoring them positive, then apply a penalty for the break itself. A crash can no longer make a name look cheaper or less stretched. 2. Raise the break threshold from 8% to 15% below the 50-day. In a broad selloff healthy names routinely sit 5-10% under their 50-day; only a real break (ON -18%, ALB -26%, MPWR -16%) trips it, while GOOG/GOOGL at -8% are spared. ON drops to mid-pack; GOOG/GOOGL keep their top spots.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a deterministic, test-pinned momentum “too-late” scorecard implementation in trading-strategies, backed by a new typed Financial Modeling Prep (FMP) read-only client in exchange, and exposes both via package exports.
Changes:
- Add a pure, deterministic scoring rubric (
computeScorecard) plus a TipRanks-only variant, both with stable sorting and unit tests. - Add
MomentumScorecardas a thin I/O orchestrator that fetches FMP inputs and delegates all scoring to the pure rubric. - Add a typed
FmpAPIclient and zod schemas, and export the new modules through package indices.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/trading-strategies/src/scorecard/MomentumScorecard.ts | I/O orchestrator that fetches FMP data per ticker and calls the pure scoring rubric |
| packages/trading-strategies/src/scorecard/index.ts | Exposes scorecard modules from the scorecard folder |
| packages/trading-strategies/src/scorecard/computeTipRanksScorecard.ts | Adds TipRanks-specific deterministic scorecard computation and banding |
| packages/trading-strategies/src/scorecard/computeTipRanksScorecard.test.ts | Unit tests for TipRanks scorecard bands and ordering |
| packages/trading-strategies/src/scorecard/computeScorecard.ts | Implements the deterministic momentum scoring rubric and forward-EPS selection |
| packages/trading-strategies/src/scorecard/computeScorecard.test.ts | Unit tests pinning scoring bands, ordering, trend-break guard, and forward-year selection |
| packages/trading-strategies/src/index.ts | Exports the new scorecard module from trading-strategies package root |
| packages/exchange/src/index.ts | Exports the new data module from exchange package root |
| packages/exchange/src/data/index.ts | New data module export surface |
| packages/exchange/src/data/fmp/schema/FmpRatingsSnapshotSchema.ts | New zod schema/type for FMP ratings snapshot |
| packages/exchange/src/data/fmp/schema/FmpQuoteSchema.ts | New zod schema/type for FMP quote |
| packages/exchange/src/data/fmp/schema/FmpPriceTargetSummarySchema.ts | New zod schema/type for FMP price target summary |
| packages/exchange/src/data/fmp/schema/FmpPriceTargetConsensusSchema.ts | New zod schema/type for FMP price target consensus |
| packages/exchange/src/data/fmp/schema/FmpAnalystEstimateSchema.ts | New zod schema/type for FMP analyst estimates |
| packages/exchange/src/data/fmp/index.ts | Exports FMP API and schema modules |
| packages/exchange/src/data/fmp/FmpAPI.ts | Implements the typed, retried FMP HTTP client |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+35
to
+39
| /** @see https://site.financialmodelingprep.com/developer/docs/stable#quote */ | ||
| async getQuote(symbol: string) { | ||
| const response = await this.#client.get('/quote', {params: {symbol}}); | ||
| return z.array(FmpQuoteSchema).parse(response.data)[0]; | ||
| } |
Comment on lines
+41
to
+45
| /** @see https://site.financialmodelingprep.com/developer/docs/stable#price-target-consensus */ | ||
| async getPriceTargetConsensus(symbol: string) { | ||
| const response = await this.#client.get('/price-target-consensus', {params: {symbol}}); | ||
| return z.array(FmpPriceTargetConsensusSchema).parse(response.data)[0]; | ||
| } |
Comment on lines
+53
to
+57
| /** @see https://site.financialmodelingprep.com/developer/docs/stable#ratings-snapshot */ | ||
| async getRatingsSnapshot(symbol: string) { | ||
| const response = await this.#client.get('/ratings-snapshot', {params: {symbol}}); | ||
| return z.array(FmpRatingsSnapshotSchema).parse(response.data)[0]; | ||
| } |
Comment on lines
+59
to
+63
| /** @see https://site.financialmodelingprep.com/developer/docs/stable#price-target-summary */ | ||
| async getPriceTargetSummary(symbol: string) { | ||
| const response = await this.#client.get('/price-target-summary', {params: {symbol}}); | ||
| return z.array(FmpPriceTargetSummarySchema).parse(response.data)[0]; | ||
| } |
Comment on lines
+162
to
+185
| function toRow(input: ScorecardInput): ScorecardRow { | ||
| const upsidePct = (input.targetConsensus / input.price - 1) * 100; | ||
| const extensionPct = (input.price / input.movingAverage200 - 1) * 100; | ||
| const forwardPE = input.epsForwardYear1 > 0 ? input.price / input.epsForwardYear1 : null; | ||
| const epsGrowthPct = input.epsForwardYear1 > 0 ? (input.epsForwardYear2 / input.epsForwardYear1 - 1) * 100 : null; | ||
| /* | ||
| * Both must be present: a zero last-month target means "no target issued recently" (missing data), | ||
| * not a 100% cut, so it scores neutral rather than getting hammered. | ||
| */ | ||
| const revisionPct = | ||
| input.targetLastMonth > 0 && input.targetLastQuarter > 0 | ||
| ? (input.targetLastMonth / input.targetLastQuarter - 1) * 100 | ||
| : 0; | ||
|
|
||
| const trendBroken = isTrendBroken(input.price, input.movingAverage50); | ||
|
|
||
| /* | ||
| * A broken trend means the analyst target is stale, so a crash makes the upside and extension bands | ||
| * look deceptively good ("cheaper, less stretched"). Don't trust them: cap their positive | ||
| * contribution at zero, then add a penalty for the break itself. | ||
| */ | ||
| const upsideScore = trendBroken ? Math.min(0, scoreUpside(upsidePct)) : scoreUpside(upsidePct); | ||
| const extensionScore = trendBroken ? Math.min(0, scoreExtension(extensionPct)) : scoreExtension(extensionPct); | ||
|
|
Comment on lines
+79
to
+89
| function toRow(input: TipRanksScorecardInput): TipRanksScorecardRow { | ||
| const upsidePct = (input.targetConsensus / input.price - 1) * 100; | ||
| const extensionPct = (input.price / input.movingAverage200 - 1) * 100; | ||
|
|
||
| const score = | ||
| scoreUpside(upsidePct) + | ||
| scoreExtension(extensionPct) + | ||
| scoreSmartScore(input.smartScore) + | ||
| scoreConsensus(input.consensus) + | ||
| scoreTrailingPE(input.trailingPE); | ||
|
|
Comment on lines
+31
to
+32
| const {epsForwardYear1, epsForwardYear2} = selectForwardEps(estimates, now); | ||
|
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new FMP client can return undefined for empty API responses (leading to confusing downstream crashes) and lacks an HTTP 429 retry condition, reducing reliability under common rate limiting.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
packages/exchange/src/data/fmp/FmpAPI.ts:39
- getQuote() returns the first element of the parsed array without handling the empty-array case. If FMP returns an empty list for an unknown/unsupported symbol, this will return undefined and fail later with a confusing "cannot read property" error.
async getQuote(symbol: string) {
const response = await this.#client.get('/quote', {params: {symbol}});
return z.array(FmpQuoteSchema).parse(response.data)[0];
}
packages/exchange/src/data/fmp/FmpAPI.ts:45
- getPriceTargetConsensus() returns the first element of the parsed array without guarding against an empty response, which can surface as undefined later rather than a clear error.
async getPriceTargetConsensus(symbol: string) {
const response = await this.#client.get('/price-target-consensus', {params: {symbol}});
return z.array(FmpPriceTargetConsensusSchema).parse(response.data)[0];
}
packages/exchange/src/data/fmp/FmpAPI.ts:57
- getRatingsSnapshot() returns the first element of the parsed array without guarding against an empty response, which can surface as undefined later rather than a clear error.
async getRatingsSnapshot(symbol: string) {
const response = await this.#client.get('/ratings-snapshot', {params: {symbol}});
return z.array(FmpRatingsSnapshotSchema).parse(response.data)[0];
}
packages/exchange/src/data/fmp/FmpAPI.ts:63
- getPriceTargetSummary() returns the first element of the parsed array without guarding against an empty response, which can surface as undefined later rather than a clear error.
async getPriceTargetSummary(symbol: string) {
const response = await this.#client.get('/price-target-summary', {params: {symbol}});
return z.array(FmpPriceTargetSummarySchema).parse(response.data)[0];
}
- Files reviewed: 16/16 changed files
- Comments generated: 1
- Review effort level: Lite
Comment on lines
+28
to
+31
| axiosRetry(this.#client, { | ||
| retries: 5, | ||
| retryDelay: retryCount => retryCount * ms('1s'), | ||
| }); |
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.
What
Turns the ad-hoc "am I too late to buy this momentum name?" analysis into deterministic, tested code. Same inputs → same ranking, every run.
Deterministic scoring core —
trading-strategies/src/scorecard/computeScorecard.tsA transparent points rubric scored per stock (cohort-independent — a stock's score never depends on the others):
computeScorecardsorts best-first with stable tiebreaks (score → upside → ticker).selectForwardEpspicks the two nearest forward fiscal years, anchored to an injectednowso it's reproducible.Orchestrator —
MomentumScorecard.tsThin I/O layer: fetches the raw figures per ticker from FMP, hands them to the pure rubric. All judgement lives in
computeScorecard.New typed FMP client —
exchange/src/data/fmp/FmpAPI.tsRead-only Financial Modeling Prep client (
quote,price-target-consensus,analyst-estimates,ratings-snapshot) with zodlooseObjectschemas, axios + retry, mirroring the existingAlpacaAPIconventions.Notes
The rubric thresholds are intentionally easy to tune in one place. Verified end-to-end against the live FMP API across the current S&P 500 momentum top 20; unit tests pin every band, loss-making handling, order-independence, and forward-year selection.