fix(scores): rewrite scoreDecayService against actual scores schema#39
fix(scores): rewrite scoreDecayService against actual scores schema#39MarvyNwaokobia wants to merge 2 commits into
Conversation
The previous implementation queried a non-existent `borrowers` table and used console.* logging. Rewrote to query the real `scores` table (user_id, current_score), added a Redis distributed lock, replaced console with winston logger, and wired the daily cron into index.ts. Closes LabsCrypt#7
ogazboiz
left a comment
There was a problem hiding this comment.
thanks for tackling this, the decay logic itself is sound (5 pts/month, Math.floor months since last repay, floored at MIN_SCORE 300, wrapped in a redis lock). but there's a real schema mismatch that defeats the purpose of the PR, plus prettier:
-
column names. the PR aims to rewrite scoreDecayService "against the actual scores schema", but it uses s.user_id and s.current_score (scoreDecayService.ts:17, and UPDATE scores SET current_score = $1 ... WHERE user_id = $2 at :45-46). running the actual migrations (1771691269865_initial-schema + 1789000000000_ensure-core-tables) produces a scores table with columns borrower, score, updated_at, no user_id / current_score. so against the migrated DB this throws "column does not exist".
important nuance: the existing scoresService.ts / scoreReconciliationService.ts on main ALSO use user_id/current_score, so there's a repo-wide inconsistency between the app code and the migrated schema, and your tests pass only because they mock the DB. please confirm the actual column names against the live/migrated schema and align this service to them (likely SELECT s.borrower AS user_id, s.score AS current_score and UPDATE scores SET score = ... WHERE borrower = ...). if the canonical schema really is user_id/current_score, point me at the migration that defines it and i'll recheck.
-
prettier fails on src/services/scoreDecayService.ts:30 and src/cron/tests/scoreDecayJob.test.ts (~9, 43). run: npx prettier --write src/services/scoreDecayService.ts "src/cron/tests/scoreDecayJob.test.ts"
heads up: CI run is gated (first PR), i'll approve it for signal. the schema one is the real blocker, want to make sure this actually runs against the DB.
if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0
Migration 1789000000000 renames user_id→borrower and current_score→score in the scores table. Use the actual column names in queries with aliases to keep the JS interface stable. Format files to pass CI lint.
ogazboiz
left a comment
There was a problem hiding this comment.
both items from last round are fixed, merging once ci is green. the column mismatch is resolved, it now selects s.borrower AS user_id, s.score AS current_score and does UPDATE scores SET score = ... WHERE borrower = ..., which matches the actual migrated schema (scores(borrower, score, updated_at)), so it runs correctly against the DB now, and prettier passes. decay logic + redis lock unchanged. good.
if you want to keep contributing, join us on Telegram: https://t.me/+DOylgFv1jyJlNzM0
ps: CI is red right now on the migrate step, but that is a pre-existing main-branch issue (a commonjs migration file under our ESM package), nothing to do with your change. once that is sorted this lands.
Summary
scoreDecayService.tsto query the realscorestable (user_id,current_score) instead of the non-existentborrowerstablescoreDecayJob.tswith Winston logger (replacingconsole.*), a Redis distributed lock, and proper error handlingstartScoreDecayCron()intoindex.ts— runs daily at 02:00 UTCsrc/cron/__tests__/and added comprehensive tests insrc/tests/src/services/README.mdDesign decisions
LoanRepaidevent incontract_eventsTest plan
npm test -- --testPathPattern=scoreDecay)Closes #7