feat(api): add a REST route + CLI mirror for loopover_check_test_evidence#6810
Conversation
…ence loopover_check_test_evidence had neither a REST route nor a CLI mirror, unlike its same-tier deterministic-lint sibling loopover_check_slop_risk, which already has full parity (POST /v1/lint/slop-risk + an in-process stdio tool). Both are rate-limit-only, pure, path-metadata computations, so the asymmetry was arbitrary. The classification/guidance logic lived as a private method in src/mcp/server.ts, so mirroring it by copy would have created a second, drifting implementation. It is extracted verbatim into the engine as buildTestEvidenceReport, beside the classifyTestCoverage/hasLocalTestEvidence/isCodeFile/isTestPath primitives it already uses. All three surfaces derive their verdict from that one function: - src/mcp/server.ts's checkTestEvidence delegates to it (behaviour-preserving; the existing MCP tool tests pass unchanged). - POST /v1/lint/test-evidence, beside /v1/lint/slop-risk and structured identically. - The loopover_check_test_evidence stdio tool, computed IN-PROCESS like the check_slop_risk sibling rather than proxying over HTTP, so coverage self-checks work fully offline. Both new input schemas mirror the MCP tool's checkTestEvidenceShape VERBATIM -- same bounds, same optionality, and `tests` as an array of strings (not free text) -- so no surface can accept an input another would reject. buildTestEvidenceReport takes readonly arrays and copies them into hasLocalTestEvidence's mutable parameters rather than passing them through. test/unit/test-evidence-report.test.ts pins the builder's own classification independently of its consumers, at and around both ratio boundaries (0.4 strong, 0.2 adequate), plus the free-text credit rule (it may only ever LIFT an absent verdict, never loosen a real one) and readonly-input safety. The route and CLI suites then assert each surface returns exactly what the builder returns; the CLI runs against a black-holed API URL, proving no round-trip. 100% line and branch coverage on every changed line across routes.ts, server.ts, and the engine builder. Closes JSONbored#6749
|
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 #6810 +/- ##
=======================================
Coverage 93.63% 93.63%
=======================================
Files 683 683
Lines 68154 68163 +9
Branches 18704 18706 +2
=======================================
+ Hits 63818 63827 +9
Misses 3350 3350
Partials 986 986
Flags with carried forward coverage won't be shown. Click here to find out more.
|
loopover-orb
Bot
added
the
gittensor:feature
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-17 07:54:35 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionPartially addressed 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.
|
![loopover-orb[bot]](https://avatars.githubusercontent.com/in/4139483?s=60&v=4){kind=small}
1 participant
What & why
loopover_check_test_evidencehad neither a REST route nor a CLI mirror — unlike its same-tier deterministic-lint siblingloopover_check_slop_risk, which already has full parity (POST /v1/lint/slop-risk+ an in-process stdio tool). Both are rate-limit-only, pure, path-metadata computations, so the asymmetry was arbitrary.One implementation, three surfaces
The classification/guidance logic lived as a private method inside
src/mcp/server.ts. Mirroring it by copy would have created a second, drifting implementation — so it's extracted verbatim into the engine asbuildTestEvidenceReport, alongside theclassifyTestCoverage/hasLocalTestEvidence/isCodeFile/isTestPathprimitives it already uses.All three surfaces now derive their verdict from that one function:
src/mcp/server.ts'scheckTestEvidencedelegates to it — behaviour-preserving, proven by the existing MCP tool tests passing unchanged.POST /v1/lint/test-evidence, placed beside/v1/lint/slop-riskand structured identically.loopover_check_test_evidencestdio tool, computed in-process like itscheck_slop_risksibling rather than proxying over HTTP — so coverage self-checks work fully offline.Parity is therefore true by construction, not by convention.
Tests
The issue asks for "a test asserting output parity between the mirrored surfaces for identical input", so that's the backbone: for every arm —
absent, free-text credit, docs-only,strong,adequate,weak— the route and the stdio tool each return exactly what the shared builder returns. The free-text credit rule is pinned specifically (it may only ever lift anabsentverdict, never loosen a real one). The CLI test runs against a black-holed API URL (http://127.0.0.1:1), proving the in-process claim. Plus zod/400 rejection on both surfaces.100% line and branch coverage on every changed line across
routes.ts,server.ts, and the engine builder — measured against the changed-line set.Fixes from the #6807 review
The reviewer was right on all counts, and the root cause was mine: I wrote the mirror schemas from assumption instead of reading
checkTestEvidenceShape.test-evidence.ts:143). I'd typed the builder'stestsasstring, but the MCP tool's shape declarestests: z.array(z.string().max(400)).max(2000).optional()— an array.hasLocalTestEvidencewants mutablestring[]. Fixed: the builder now takesreadonly string[]and copies intohasLocalTestEvidence's mutable params. This also means my REST/CLI schemas were wrong in a way that broke the very parity the issue asks for — both now mirrorcheckTestEvidenceShapeverbatim (same bounds, same optionality), including accepting an emptychangedPathsexactly like the tool does..length-on-string. Gone; the array typing makes the intent explicit rather than relying onstringandarrayboth having.length(which is precisely why my tests passed while the types were wrong).test/unit/test-evidence-report.test.ts, a standalone suite pinning the builder independently of both consumers: exact classifications at and around both ratio boundaries (0.4 strong, 0.2 adequate), the free-text credit rule, guidance arms, and readonly-input safety.I also fixed my own process gap:
tsc --noEmitis clean this time (I'd skipped it, which is exactly why a type error reached CI).Closes #6749