chore: address review feedback on predicted rating logging (#11)
- align diagnostic endpoint date windows with getNextPDGAUpdateDate - pass pdgaNumber to initial target-rating calculation for log attribution - avoid double log when lazy-recompute returns 0 - log full error object in admin endpoint catch
This commit is contained in:
@@ -7,7 +7,7 @@ const { getOfficialRatingHistory, getOptimizedPlayerRounds } = require('../scrap
|
|||||||
const { launchBrowser } = require('../scrapers/browser');
|
const { launchBrowser } = require('../scrapers/browser');
|
||||||
const { getPlayerDataFromDB, scrapePDGARating, getAllRatingsFromDB, refreshAllPlayersInDB, getPredictedRatingFromDB, formatDisplayDate } = require('../services/player-service');
|
const { getPlayerDataFromDB, scrapePDGARating, getAllRatingsFromDB, refreshAllPlayersInDB, getPredictedRatingFromDB, formatDisplayDate } = require('../services/player-service');
|
||||||
const { getTopbarLocals } = require('../services/topbar-service');
|
const { getTopbarLocals } = require('../services/topbar-service');
|
||||||
const { calculatePredictedRating } = require('../services/rating-calculator');
|
const { calculatePredictedRating, getNextPDGAUpdateDate } = require('../services/rating-calculator');
|
||||||
const { calculateRequiredAverage } = require('../services/target-rating-calculator');
|
const { calculateRequiredAverage } = require('../services/target-rating-calculator');
|
||||||
const logger = require('../logger');
|
const logger = require('../logger');
|
||||||
|
|
||||||
@@ -449,9 +449,9 @@ router.get('/api/admin/player-state/:pdgaNumber', async (req, res) => {
|
|||||||
return res.status(404).json({ error: 'Player not found', pdgaNumber: parseInt(pdgaNumber) });
|
return res.status(404).json({ error: 'Player not found', pdgaNumber: parseInt(pdgaNumber) });
|
||||||
}
|
}
|
||||||
const rounds = await getRoundHistoryFromDB(pdgaNumber);
|
const rounds = await getRoundHistoryFromDB(pdgaNumber);
|
||||||
const now = new Date();
|
const cutoff = getNextPDGAUpdateDate();
|
||||||
const twelveMonthsAgo = new Date(now); twelveMonthsAgo.setFullYear(now.getFullYear() - 1);
|
const twelveMonthsAgo = new Date(cutoff); twelveMonthsAgo.setFullYear(cutoff.getFullYear() - 1);
|
||||||
const twentyFourMonthsAgo = new Date(now); twentyFourMonthsAgo.setFullYear(now.getFullYear() - 2);
|
const twentyFourMonthsAgo = new Date(cutoff); twentyFourMonthsAgo.setFullYear(cutoff.getFullYear() - 2);
|
||||||
|
|
||||||
const roundsInLast12mo = rounds.filter(r => new Date(r.date) >= twelveMonthsAgo).length;
|
const roundsInLast12mo = rounds.filter(r => new Date(r.date) >= twelveMonthsAgo).length;
|
||||||
const roundsInLast24mo = rounds.filter(r => new Date(r.date) >= twentyFourMonthsAgo).length;
|
const roundsInLast24mo = rounds.filter(r => new Date(r.date) >= twentyFourMonthsAgo).length;
|
||||||
@@ -468,6 +468,7 @@ router.get('/api/admin/player-state/:pdgaNumber', async (req, res) => {
|
|||||||
cutoffRating: player.cutoff_rating,
|
cutoffRating: player.cutoff_rating,
|
||||||
lastUpdated: player.last_updated,
|
lastUpdated: player.last_updated,
|
||||||
lastRoundUpdate: player.last_round_update,
|
lastRoundUpdate: player.last_round_update,
|
||||||
|
cutoffDate: cutoff.toISOString(),
|
||||||
roundCount: rounds.length,
|
roundCount: rounds.length,
|
||||||
roundsInLast12mo,
|
roundsInLast12mo,
|
||||||
roundsInLast24mo,
|
roundsInLast24mo,
|
||||||
@@ -475,7 +476,7 @@ router.get('/api/admin/player-state/:pdgaNumber', async (req, res) => {
|
|||||||
newestRound: dates[dates.length - 1] ?? null
|
newestRound: dates[dates.length - 1] ?? null
|
||||||
});
|
});
|
||||||
} catch (err) {
|
} catch (err) {
|
||||||
logger.error({ err: err.message, pdgaNumber }, 'admin player-state endpoint failed');
|
logger.error({ err, pdgaNumber }, 'admin player-state endpoint failed');
|
||||||
res.status(500).json({ error: 'Failed to fetch player state', details: err.message });
|
res.status(500).json({ error: 'Failed to fetch player state', details: err.message });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
@@ -519,7 +520,7 @@ router.post('/api/calculate-target-rating/:pdgaNumber', async (req, res) => {
|
|||||||
competition: r.competition_name
|
competition: r.competition_name
|
||||||
}));
|
}));
|
||||||
|
|
||||||
const result = calculateRequiredAverage(roundRatings, target, numRounds);
|
const result = calculateRequiredAverage(roundRatings, target, numRounds, pdgaNum);
|
||||||
|
|
||||||
logger.info(`Target rating calc for PDGA ${pdgaNum}: target=${target} rounds=${numRounds} -> avg=${result.requiredAverage}`);
|
logger.info(`Target rating calc for PDGA ${pdgaNum}: target=${target} rounds=${numRounds} -> avg=${result.requiredAverage}`);
|
||||||
|
|
||||||
|
|||||||
@@ -42,7 +42,9 @@ async function getPlayerDataFromDB(pdgaNumber, { includeMonthlyHistory = true }
|
|||||||
let stdDev = cachedPlayer.std_dev;
|
let stdDev = cachedPlayer.std_dev;
|
||||||
let excludedRoundsCount = cachedPlayer.excluded_rounds_count;
|
let excludedRoundsCount = cachedPlayer.excluded_rounds_count;
|
||||||
let cutoffRating = cachedPlayer.cutoff_rating;
|
let cutoffRating = cachedPlayer.cutoff_rating;
|
||||||
|
let recomputeAttempted = false;
|
||||||
if (!predictedRating || predictedRating === 0) {
|
if (!predictedRating || predictedRating === 0) {
|
||||||
|
recomputeAttempted = true;
|
||||||
logger.debug({ pdgaNumber, dbValue: cachedPlayer.predicted_rating }, 'lazy recompute triggered for predicted_rating');
|
logger.debug({ pdgaNumber, dbValue: cachedPlayer.predicted_rating }, 'lazy recompute triggered for predicted_rating');
|
||||||
predictedRating = await getPredictedRatingFromDB(pdgaNumber);
|
predictedRating = await getPredictedRatingFromDB(pdgaNumber);
|
||||||
const updatedPlayer = await getPlayerFromDB(pdgaNumber);
|
const updatedPlayer = await getPlayerFromDB(pdgaNumber);
|
||||||
@@ -56,8 +58,9 @@ async function getPlayerDataFromDB(pdgaNumber, { includeMonthlyHistory = true }
|
|||||||
|
|
||||||
const rating = cachedPlayer.current_rating;
|
const rating = cachedPlayer.current_rating;
|
||||||
const rawRatingChange = cachedPlayer.rating_change;
|
const rawRatingChange = cachedPlayer.rating_change;
|
||||||
if (predictedRating != null && predictedRating <= 0) {
|
// Only warn about ≤0 if it wasn't already explained by a failed recompute (which has its own log)
|
||||||
logger.warn({ pdgaNumber, dbValue: predictedRating }, 'predicted rating present but <= 0 — rendered as empty');
|
if (!recomputeAttempted && predictedRating != null && predictedRating <= 0) {
|
||||||
|
logger.warn({ pdgaNumber, dbValue: predictedRating }, 'predicted rating present but <= 0 in DB without recompute — rendered as empty');
|
||||||
}
|
}
|
||||||
const resolvedPredicted = predictedRating > 0 ? predictedRating : null;
|
const resolvedPredicted = predictedRating > 0 ? predictedRating : null;
|
||||||
const resolvedStdDev = stdDev > 0 ? stdDev : null;
|
const resolvedStdDev = stdDev > 0 ? stdDev : null;
|
||||||
|
|||||||
@@ -1,14 +1,14 @@
|
|||||||
const { calculatePredictedRating, getNextPDGAUpdateDate } = require('./rating-calculator');
|
const { calculatePredictedRating, getNextPDGAUpdateDate } = require('./rating-calculator');
|
||||||
const logger = require('../logger');
|
const logger = require('../logger');
|
||||||
|
|
||||||
function calculateRequiredAverage(roundRatings, targetRating, numRounds) {
|
function calculateRequiredAverage(roundRatings, targetRating, numRounds, pdgaNumber) {
|
||||||
if (!Array.isArray(roundRatings) || roundRatings.length === 0) {
|
if (!Array.isArray(roundRatings) || roundRatings.length === 0) {
|
||||||
const err = new Error('No round history');
|
const err = new Error('No round history');
|
||||||
err.code = 'NO_ROUNDS';
|
err.code = 'NO_ROUNDS';
|
||||||
throw err;
|
throw err;
|
||||||
}
|
}
|
||||||
|
|
||||||
const currentPredicted = calculatePredictedRating(roundRatings, { callsite: 'target-rating-calculator' }).rating;
|
const currentPredicted = calculatePredictedRating(roundRatings, { pdgaNumber, callsite: 'target-rating-calculator' }).rating;
|
||||||
const nextUpdate = getNextPDGAUpdateDate();
|
const nextUpdate = getNextPDGAUpdateDate();
|
||||||
const syntheticDate = new Date(nextUpdate.getTime() - 24 * 60 * 60 * 1000);
|
const syntheticDate = new Date(nextUpdate.getTime() - 24 * 60 * 60 * 1000);
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user