fix(engine): correct computeOpportunityFreshness's contract and cover the non-finite clock - #9626
Conversation
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 01:38:12 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
… the non-finite clock (JSONbored#9618) Its JSDoc claimed a [0.05, 1] range and said it mirrors `opportunityFreshnessFactor` in `src/signals/reward-risk.ts` — both wrong: two paths return the 0 no-signal sentinel (no open issues, or a non-finite clock), and JSONbored#8011 made `opportunityFreshnessFactor` in `packages/loopover-engine/src/reward-risk.ts` delegate TO this module, so this is the canonical implementation, not a mirror. The JSDoc now states the real 0-sentinel cases and the [0.05, 1] measured range (and that 0 differs 20x from 0.05 in the ranker score). The `/* v8 ignore next */` on the non-finite-`nowMs` guard was also false — that guard is reachable from reward-risk.ts:289's `args.nowMs ?? Date.now()` — so it's removed and the branch is now covered by a real test. No behaviour change: the two 0 returns, the clamp(exp(-age/20), 0.05, 1) formula, round4, and pickTimestamp order are untouched. Tests (engine node:test suite): non-finite clock -> 0, no open issues -> 0, and the exact measured floor/ceiling (0.05 / 1). The root vitest suite already covers the non-finite branch, so both the engine and backend Codecov flags union to cover it. Closes JSONbored#9618 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9626 +/- ##
==========================================
- Coverage 90.02% 90.01% -0.01%
==========================================
Files 888 888
Lines 111983 111986 +3
Branches 26570 26571 +1
==========================================
- Hits 100810 100809 -1
Misses 9843 9843
- Partials 1330 1334 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Closes #9618
What
computeOpportunityFreshnessinpackages/loopover-engine/src/opportunity-freshness.tshad a JSDoc that was wrong in three ways and a false coverage suppression:[0.05, 1], but two paths return0(no open issues, or a non-finitenowMs).0is a meaningful sentinel — 20x below the0.05floor in the multiplicative ranker score — so the JSDoc now names both0cases and distinguishes0(no signal) from0.05(measured, maximally stale).opportunityFreshnessFactorinsrc/signals/reward-risk.ts(no such function there) and said this module mirrors it. opportunityFreshnessFactor still hand-duplicates its arithmetic and reads live Date.now(), unlike opportunityCompetitionFactor which #7529 converted to delegate to the pure mirror #8011 madeopportunityFreshnessFactorinpackages/loopover-engine/src/reward-risk.tsdelegate to this module — so the reference is corrected and the direction reworded (this is the canonical implementation), matching theopportunityCompetitionFactordelegation comment./* v8 ignore next */on the!Number.isFinite(nowMs)guard claimed the branch was unreachable, but it's reachable fromreward-risk.ts:289'sargs.nowMs ?? Date.now(). Removed, and the branch is now covered by a real test.No behaviour change: the two
0returns, theclamp(exp(-age/20), 0.05, 1)formula,round4, and thepickTimestamporder are byte-for-byte unchanged (the diff is JSDoc + the removed suppression comment only).Tests
New named tests in the engine's own node:test suite (
packages/loopover-engine/test/opportunity-freshness.test.ts): a non-finite clock (NaN/Infinity) →0, no open issues →0, and the exact measured floor/ceiling (0.05/1). The root vitest suite already exercised the non-finite branch, so both theengineandbackendCodecov flags union to cover it. Verified: engine suite 10/10, root suite 4/4,tsc --noEmitclean,git diff --checkclean.