fix(engine): drop opportunity-metadata's blanket v8-ignore directives and branch-cover the scoring logic they hid - #9635
Conversation
… and branch-cover the scoring logic they hid
opportunity-metadata.ts carried v8-ignore suppressions across nearly
all of its decision logic -- computeMetadataPotential wholesale, the
feasibility clock guard and title tiers, titlesOverlap, all five
computeMetadataDupRisk branches, and six bare unexplained directives --
in a file the engine Codecov flag is supposed to grade. The rationales
were self-refuting ("exercised in ranker tests": if true the directive
is redundant, if false it conceals an untested branch), and stripping
them proves the point: four partial branches surface under the
existing ranker suites alone.
Delete every directive except the opportunityMetadataInternals
test-only export block's legitimate pair. No production logic changes
-- the diff to the source file is 27 pure comment-line deletions; every
formula, threshold, label list, and return expression is byte-for-byte
identical. Direct unit tests now pin each previously-suppressed
branch: label tiers and normalization, the non-finite clock guard,
title-length tiers, titlesOverlap's emptiness/equality/containment-
threshold/swap arms, dup-risk's self/cross-repo/overlap filters, the
competition-context and timestamp ?? arms, the targetability filter,
and the defensive re-parse and clamp01 guards (exercised by making the
underlying primitive disagree, the only way without a logic change).
Regression tests land in BOTH graded suites: the engine package's own
node:test suite (the `engine` Codecov flag's coverage run) and the
root vitest unit suite (the `backend` flag). Under each flag's own
lcov the file now reports zero uncovered lines and zero partial
branches outside the retained internals block.
Closes JSONbored#9616
|
Warning ⏸️ LoopOver review result - manual review recommendedReview updated: 2026-07-29 02:51:34 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #9635 +/- ##
==========================================
+ Coverage 90.12% 90.14% +0.02%
==========================================
Files 891 891
Lines 112306 112279 -27
Branches 26629 26680 +51
==========================================
+ Hits 101216 101218 +2
+ Misses 9760 9727 -33
- Partials 1330 1334 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Closes #9616
Summary
fix(engine): drop opportunity-metadata's blanket v8-ignore directives and branch-cover the scoring logic they hidpackages/loopover-engine/src/opportunity-metadata.tscomputes the five ranker inputs that decide which issue the miner works next, yetv8 ignoredirectives suppressed branch counting across nearly all of its decision logic —computeMetadataPotentialwholesale (plus per-branch ignores), the feasibility clock guard and title-length tiers,titlesOverlap, all fivecomputeMetadataDupRiskbranches, and six bare unexplained directives — in a filecoverage.includeand theengineCodecov flag are supposed to grade. The existing rationales are self-refuting: if a line is genuinely exercised,v8 ignoreon it is redundant; the directive only changes anything when the line is NOT hit.Root cause / RED evidence
Stripping the directives without adding tests proves they were concealing untested branches: under the existing ranker suites (
miner-opportunity-ranker,opportunity-metadata-signals,miner-opportunity-fanout) the file immediately reports four partial-branch lines (46, 87, 96, 191) — including theclamp01guard, the timestamp fallbacks, and the??-arm on the competition context. Scoring rules like therefactorbonus and the same-repo peer filter could have been inverted without any test failing.Fix
v8 ignoredirective in the file — thestart/stoppairs, the per-branchnexts, and the six bare ones — except the legitimateopportunityMetadataInternalstest-only export block, which keeps its single pair. The file now contains exactly onev8 ignorepair and no other directive.git show --numstat:0 27). Every numeric literal, label array, comparison operator, and return expression is byte-for-byte identical.packages/loopover-engine/test/opportunity-metadata.test.ts(node:test +node:assert/strict, importing from../dist/index.js/../dist/opportunity-metadata.jsfor the internals, per theopportunity-ranker.test.tsstyle) and a mirrored newtest/unit/opportunity-metadata.test.ts(vitest) so thebackendflag grades the same arms. They call the exported functions andopportunityMetadataInternalsmembers directly, not onlyrankMetadataOpportunities.Test coverage delivered (every enumerated branch)
computeMetadataPotential: negative label → 0 (also alongside a positive label), positive label (+0.35),"bug"(+0.1),"refactor"(+0.05),"bug"+"refactor", neutral-only baseline 0.45; label normalization (trim/lowercase/non-string/blank drops).computeMetadataFeasibility: non-finitenowMs(NaN and Infinity → 0); title tiers>= 8/4-7/< 4on the normalized title (incl. whitespace collapse); comment-load zeroing at 25; non-finite and negativecommentsCountnormalized to 0; no-timestamp stale sentinel; unparseableupdatedAtfalling back to validcreatedAt.issueAgeDaysre-parse guard (Date.parse stubbed to differ between the validation call and the re-parse call) and theclamp01non-finite guard (Math.exp stubbed to NaN) — the only branches unreachable through natural inputs, which is exactly why the old "exercised in ranker tests" rationales could never have been true for them.opportunityMetadataInternals.titlesOverlap: empty left, empty right, exact equality, containment withshorter.length >= 12, containment with< 12, both swap arms, and no-containment.computeMetadataDupRisk: blank/whitespace title → 1, self-peer skip (with repo case/whitespace variance), same-number-different-repo falling through to the cross-repo skip, cross-repo skip, same-repo overlapping peer (1 → 0.5, 2 → 2/3), zero-overlap batch → 0.buildMetadataRankInput:highRiskDuplicateClusters/openPullRequestspresent (dupRisk = 0.5 from 2/4) and absent (?? 0arms, dupRisk 0);updatedAt/createdAtpresent and absent (?? nullarms).rankMetadataOpportunities: ranks and orders targetable candidates; drops a repo whose goal spec setsminerEnabled: false(both filter arms).normalizeLabels,pickMetadataTimestamp(all fallback arms),resolveGoalSpec(match with case/whitespace variance, non-match, absent map).Gates (verbatim)
Engine package suite (the
engineflag's own suite, +29 tests):# tests 833 / # pass 833 / # fail 0.engineflag — CI's exact command (node --experimental-strip-types scripts/engine-coverage.ts), lcov cross-checked:backendflag — root vitest v8 coverage, same cross-check:(The only remaining
v8 ignorepair is the retained internals export block.)Validation commands (repo root)