test(scoring): pin the unpinned-fallback and languages-fetch-failed warnings#8526
Conversation
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
…arnings refreshScoringModelSnapshot pushes two operator-facing warnings that nothing asserted. The unpinned-ref warning was already triggered by the SHA-failure test but never checked, and it is the only signal that scoring is running against a mutable ref. The languages-fetch-failed branch had never executed at all: tests reaching constantsUsable === true always stubbed programming_languages.json to succeed, and tests where it failed had constants.py fail first, short-circuiting earlier. Adds the missing assertion and a constants-succeed/languages-fail case covering the warning and the empty programmingLanguages fallback. Verified both fail when their warning push is removed. Closes JSONbored#8327
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 18:31:00 UTC
Review summary Nits — 2 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.
|
Summary
refreshScoringModelSnapshot(src/scoring/model.ts) pushes two operator-facing warnings that nothing asserted:1. The unpinned-fallback warning (~line 59). The existing "is fail-open when the upstream SHA fetch fails" test already drives this branch (its SHA endpoint throws) and checks
payload.upstreamSourceShais undefined — but never looked atwarnings. That string is the only signal that scoring is currently running unpinned against a mutable ref, where an upstream force-push silently changes what every repo scores against. Added the assertion to the existing test.2. The languages-fetch-failed branch (~line 116-117) had never executed. Every test reaching
constantsUsable === truealso stubbedprogramming_languages.jsonto succeed, and every test where languages failed hadconstants.pyfail first — short-circuiting into the earlier!constantsUsablepath before this line. Added a test combining constants succeed + languages 404, assertingsourceKind === "raw-github",programmingLanguagesdegrades to{}, and the warning text is present.Verified both tests actually pin their branches rather than passing alongside them — deleting each
warnings.push(...)makes the corresponding test fail, restoring it makes all 89 pass:Programming language weights fetch failedpushOne correction to the issue's text: it suggests asserting
programmingLanguageson the payload. It is a top-level field of the snapshot record (ScoringModelSnapshotRecord.programmingLanguages), notpayload.programmingLanguages, so the assertion targets the real field.Closes #8327
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
tsc --noEmitis clean andgit diff --checkpasses.codecov/patchhas no changed production lines to score. The scoped coverage run emits a non-emptycoverage/lcov.infocontainingsrc/scoring/model.ts(the suite imports it directly), satisfying the "Verify coverage report exists" step.test/unit/scoring.test.tspasses in full — 89 tests.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.UI Evidence
Not applicable — unit-test additions; no visible UI, frontend, docs, or extension change.