feat(fantasy): use FantasyCalc player values for trade quantification

[sfreeman422]

Summary

Trade recommendations were being generated using only roster context and AI judgment, with no consensus player valuation signal — leading to unfair suggestions per veteran/expert feedback. This adds FantasyCalc player values as a quantitative fairness check and tightens the trade prompt to use them explicitly.

Changes

• FantasyCalc integration: new getFantasyCalcValues() fetches consensus player values from https://api.fantasycalc.com/values/current, scaled to each league's actual format (resolveLeagueFormat() derives dynasty/keeper vs redraft, QB count/superflex, and PPR from Sleeper league settings). Cached 6h per format; fetch failures are non-fatal and logged.
• FantasyPlayer now carries marketValue and positionRank, populated across rosters, waiver candidates, pending trades, and pending waivers.
• Prompt improvements: the ChatGPT trade-analysis prompt now explains what marketValue/positionRank mean, requires summing give/receive value per proposed trade, keeping totals within ~10–20% (with a slight edge to the user), and requires the rationale to state the value comparison explicitly instead of relying on qualitative judgment alone.
• Frontend FantasyPlayer type updated to match for type safety.

Testing

• Added unit tests for resolveLeagueFormat, getFantasyCalcValues (caching + failure handling).
• Updated existing fixtures for the new FantasyPlayer fields.
• Full backend (729 tests) and frontend (116 tests) suites pass, including coverage gates.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

🟡 Changes recommended

Moderate cache, failure-handling, and prompt issues remain, along with missing integration coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR integrates FantasyCalc consensus values into fantasy trade quantification, propagating player valuations through models and AI trade analysis.

Changes:

• Adds format-aware FantasyCalc fetching, caching, and failure handling.
• Adds marketValue and positionRank across backend and frontend player data.
• Updates trade-analysis prompts to require quantitative value comparisons.
• Adds format, caching, and failure-handling tests.

Review findings:

• fantasy.service.ts:399 — Moderate, 3 votes: Include normalized numTeams in the cache key.
• fantasy.service.ts:212 — Nit, 2 votes: Add successful overview integration coverage for valuation propagation.
• fantasy.service.ts:428 — Moderate, 1 vote: Add negative caching or retry backoff for FantasyCalc failures.
• fantasy.service.ts:1252 — Moderate, 1 vote: Clarify that values use the applied league-format scaling.

File summaries

File	Description
packages/frontend/src/app.model.ts	Adds valuation fields to frontend player types.
packages/backend/src/fantasy/fantasy.service.ts	Integrates FantasyCalc values, enriches players, and updates prompts.
packages/backend/src/fantasy/fantasy.service.spec.ts	Tests format resolution, caching, and failure handling.
packages/backend/src/fantasy/fantasy.model.ts	Defines valuation and player data types.

Review details

Suppressed comments (2)

packages/backend/src/fantasy/fantasy.service.ts:430

• The non-fatal path returns an empty map without recording the failure, but getOverview awaits this promise inside its Promise.all. During a FantasyCalc timeout, every overview request can therefore add up to 10 seconds and issue another upstream request, amplifying an outage. Add a short-lived negative cache or retry backoff (while still allowing recovery) so this dependency cannot impose the timeout on every page load.

    } catch (error) {
      logError(this.serviceLogger, 'Failed to load FantasyCalc player values', error, { isDynasty, numQbs, ppr });
      return new Map();

packages/backend/src/fantasy/fantasy.service.ts:1252

• This instruction describes the totals as using "redraftValue-equivalent" scaling, but marketValue is populated from entry.value for dynasty/keeper leagues and only from entry.redraftValue for redraft. That wording gives the model a contradictory basis for comparing dynasty trades; describe it as the league-format scaling already applied.

        'must be within roughly 10-20% of each other (using redraftValue-equivalent scaling already applied) so the target ' +

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🟡 Changes recommended

Moderate cache, failure-handling, and prompt issues remain, along with missing integration coverage....

Addressed the requested fixes in d20be0a.

[Copilot]

🟡 Changes recommended

Unresolved findings affect valuation caching, format handling, cache freshness, server-side fairness enforcement, and outage-state handling.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (5)

packages/backend/src/fantasy/fantasy.service.ts:213

• Although getFantasyCalcValues catches failures, this optional dependency is awaited inside the overview's critical Promise.all and has a 10-second timeout. A slow or unavailable FantasyCalc endpoint can therefore delay every cold-cache overview response by up to 10 seconds before usable Sleeper/ESPN data is returned; keep the fallback out of the critical path or use a tighter bounded timeout.

      this.getFantasyCalcValues(league),

packages/backend/src/fantasy/fantasy.service.ts:1266

• This requirement is only sent as an instruction to the model; no server-side check enforces it. validateAnalysis accepts any player IDs and values, and buildTradeSuggestions only verifies roster membership, so an otherwise valid model response can still expose a trade with a large market-value imbalance (or null values treated inconsistently). Compute the give/receive totals from the roster marketValues and reject or omit suggestions that violate the fairness rule before returning them.

        "Sum the marketValue of givePlayerIds and receivePlayerIds for both sides of each suggestion: the two sides' totals " +
        "must be within roughly 10-20% of each other (using the league-format scaling already applied to each player's marketValue) so the target " +
        "manager is realistically likely to accept, while keeping a small, subtle edge in the user's favor - never propose a " +
        "trade where the user's outgoing marketValue total is more than about 20% below what they receive. State the " +

packages/backend/src/fantasy/fantasy.service.ts:428

• The new values are cached for 6 hours, but getTradeAnalysis continues to cache the generated prompt result for 24 hours. After the FantasyCalc cache refreshes, a normal overview can therefore show current player values while its trade suggestions and insights were generated with values up to 18 hours old. Tie the analysis cache to the market-value snapshot or invalidate/regenerate it when the values refresh.

      this.fantasyCalcCache.set(cacheKey, { expiresAt: Date.now() + FANTASYCALC_CACHE_MS, values });

packages/backend/src/fantasy/fantasy.service.ts:385

• FantasyCalc only accepts numQbs values of 1 or 2 and ppr values of 0, 0.5, or 1, but this forwards arbitrary Sleeper settings. A 2-QB plus superflex league yields numQbs = 3, and custom PPR leagues can send values such as 0.25; the API request will then fail and the catch path silently returns an empty valuation map. Normalize to a supported format or explicitly disable the valuation fetch for unsupported settings.

    const numQbs = Math.max(1, rosterPositions.filter((slot) => slot === 'QB' || slot === 'SUPER_FLEX').length);
    const ppr = league.scoring_settings?.rec ?? 0.5;

packages/backend/src/fantasy/fantasy.service.ts:1257

• The failure path in getFantasyCalcValues returns an empty map, so a provider outage makes every player's marketValue null, not just unranked players. This prompt therefore misclassifies a temporary feed failure and may lead the model to claim that no players are ranked; pass valuation availability separately or state that null also means the feed was unavailable.

        'at the same position, 1 is best). A null marketValue means the player is unranked by FantasyCalc (e.g. a rookie ' +
        'or deep bench piece); in that case fall back to position, roster needs, and matchup context instead of value. ' +

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved findings affect valuation caching, format handling, cache freshness, server-side fairness enforcement, and outage-state handling....

Addressed in 9d479d5.

[Copilot]

🟡 Changes recommended

A critical TypeScript issue prevents test compilation, and additional fallback, validation, and cache-invalidation issues remain.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

packages/backend/src/fantasy/fantasy.service.ts:1126

• The new fairness check sums the AI-provided arrays as-is, but neither the schema nor validateAnalysis enforces unique IDs. A response such as ['give-fair', 'give-fair'] can therefore inflate the outgoing total and pass this gate while returning the same player twice; reject duplicate or unknown IDs before calculating the totals.

      const giveTotal = this.totalMarketValue(give);
      const receiveTotal = this.totalMarketValue(receive);

• Files reviewed: 4/4 changed files
• Comments generated: 3
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

A critical TypeScript issue prevents test compilation, and additional fallback, validation, and cache-invalidation issues remain....

Addressed the remaining items from this review in 0a53fc1.

[Copilot]

🟡 Changes recommended

Three unresolved moderate findings cover cache growth, blocking provider fetches, and incorrect redraft value selection.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

packages/backend/src/fantasy/fantasy.service.ts:216

• Because this optional fetch is awaited in the overview's Promise.all, a cold-cache miss or negative-cache expiry blocks the entire page response for up to FANTASYCALC_TIMEOUT_MS when FantasyCalc is unresponsive, despite failures being described as non-fatal. Consider serving the overview without waiting for this provider (or using stale values) and refreshing valuations independently.

      this.getFantasyCalcValues(league),

packages/backend/src/fantasy/fantasy.service.ts:440

• value is the FantasyCalc market value for the requested league format, while redraftValue is a separate response field. Selecting redraftValue for every redraft league discards the value calculated for the requested numQbs/numTeams/ppr parameters, so redraft marketValue can be inconsistent with the format sent in the request. Use entry.value here; isDynasty already selects the requested dynasty/redraft format, and update the redraft expectations in the spec accordingly.

                value: isDynasty ? entry.value : entry.redraftValue,

• Files reviewed: 4/4 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Three unresolved moderate findings cover cache growth, blocking provider fetches, and incorrect redraft value selection....

Implemented in commit 3fca945: overview now avoids blocking on FantasyCalc misses by using cached/stale fallback with bounded wait, redraft market values now use the format-specific entry.value, and analysis cache entries are now replaced per logical key to prevent unbounded growth.

[Copilot]

🟡 Changes recommended

Unresolved backend issues affect cache correctness, fallback behavior, and zero-value fairness validation.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

Previously missed (1) — in code that hasn't changed since the last review.

packages/backend/src/fantasy/fantasy.service.ts:1189

• This receiveTotal > 0 guard skips the fairness comparison when a valid FantasyCalc entry has value 0. A suggestion can therefore give a positive-value player for a zero-value player and still pass the server-side gate; only null should disable the quantitative check.

• Files reviewed: 4/4 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

Unresolved backend issues affect cache correctness, fallback behavior, and zero-value fairness validation....

Fixed in 9205a57.

[Copilot]

🔵 Needs a closer look

Resolve the analysis-cache format identity and stale-generation caching issues.

Review details

Suppressed comments (3)

packages/backend/src/fantasy/fantasy.model.ts:103

• These public model comments say null means only that a player is unranked, but toPlayer also emits null whenever the non-fatal FantasyCalc feed is unavailable. That distinction is documented in the AI prompt and matters to API consumers, so update both field comments to include the unavailable-feed case.

  /** Consensus market value from FantasyCalc for the league's format (dynasty/redraft, QB count, PPR). Null if unranked. */
  marketValue: number | null;
  /** FantasyCalc rank among players at the same position. Null if unranked. */
  positionRank: number | null;

packages/backend/src/fantasy/fantasy.service.ts:347

• marketValueSnapshot.version is only a counter scoped by getFantasyCalcCacheKey, but the analysis cache key above does not include that format key. If a league changes format and the new format reaches the same numeric version, this reuses an analysis generated from the old format's values. Include the format/cache key in the analysis-cache identity (or make the version globally unique) so format changes cannot reuse stale trade analysis.

        marketValueSnapshot.version,

packages/backend/src/fantasy/fantasy.service.ts:572

• Because FantasyCalc refreshes can complete while an overview is still generating, an older generation can finish after one using a newer marketValueVersion and overwrite the cache here with stale analysis. The next request then detects the mismatch and regenerates, causing avoidable OpenAI calls; only replace the entry when the current cached version is not newer than this generation's version.

      marketValueVersion,

• Files reviewed: 4/4 changed files
• Comments generated: 0 new
• Review effort level: Lite