Skip to content

JMCP: Papers - jmcp/extract-recommendation-scoring-weights-i-AnLIKEry#8

Closed
landigf wants to merge 1 commit into
mainfrom
jmcp/extract-recommendation-scoring-weights-i-AnLIKEry
Closed

JMCP: Papers - jmcp/extract-recommendation-scoring-weights-i-AnLIKEry#8
landigf wants to merge 1 commit into
mainfrom
jmcp/extract-recommendation-scoring-weights-i-AnLIKEry

Conversation

@landigf

@landigf landigf commented Mar 24, 2026

Copy link
Copy Markdown
Owner

Review: Extract recommendation scoring weights into config

Change summary: Moves hard-coded numeric weights from scoreFeedPaper and scoreTrendingPaper in packages/db into two named as const config objects (feedScoringWeights, trendingScoringWeights) exported from packages/contracts. The scoring functions now reference w.<field> instead of magic numbers. No logic changes — all values are identical to the originals.

Validation confidence: High

  • Pure mechanical extraction — every literal maps 1:1 to the config field with the same value (5, 4, 3, 2, 2, 24 for feed; 4, 3, 3, 16 for trending).
  • as const satisfies Record<string, number> gives both literal types and structural safety.
  • No new runtime code paths; scoring formulas are structurally identical.

Risks that still remain:

  1. ScoringWeights union type is loose — it's a union of two structurally different objects (feedScoringWeights has interestMatch/topicOverlap that trending lacks). Consumers accepting ScoringWeights won't get compile-time safety on field access without narrowing. If no consumer uses this type today it's harmless, but consider making it a discriminated union or splitting into two explicit types if you plan to pass weights generically.
  2. No tests exercise the weights indirectly — if the scoring functions already have test coverage, this is fine. If not, a future typo in the config (e.g. changing a weight thinking it's one scorer but it's the other) would go undetected.
  3. recencyMaxDays is a ceiling, not a weight — naming it alongside multiplicative weights could mislead a future contributor. A comment or grouping would help, though low priority.

Overall: clean, low-risk refactor. Good to merge after npm run test && npm run check passes.

@landigf

landigf commented Mar 25, 2026

Copy link
Copy Markdown
Owner Author

Closing: will integrate valuable features directly on main to avoid cascade merge conflicts.

@landigf landigf closed this Mar 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant