Skip to content

feat(api): add a REST route + CLI mirror for loopover_check_test_evidence#6807

Closed
davion-knight wants to merge 1 commit into
JSONbored:mainfrom
davion-knight:feat-test-evidence-mirror
Closed

feat(api): add a REST route + CLI mirror for loopover_check_test_evidence#6807
davion-knight wants to merge 1 commit into
JSONbored:mainfrom
davion-knight:feat-test-evidence-mirror

Conversation

@davion-knight

Copy link
Copy Markdown
Contributor

What & why

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.

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 as buildTestEvidenceReport, alongside the classifyTestCoverage / hasLocalTestEvidence / isCodeFile / isTestPath primitives it already uses.

All three surfaces now derive their verdict from that one function:

  • src/mcp/server.ts's checkTestEvidence delegates to it — behaviour-preserving, proven by the existing MCP tool tests passing unchanged.
  • POST /v1/lint/test-evidence, placed beside /v1/lint/slop-risk and structured identically.
  • The loopover_check_test_evidence stdio tool, computed in-process like its check_slop_risk sibling 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 armabsent, 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 an absent verdict, 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.

Closes #6749

…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 inside src/mcp/server.ts as a private method, so mirroring it
by copy would have created a second, drifting implementation. Instead it is extracted verbatim into
the engine as buildTestEvidenceReport (packages/loopover-engine/src/signals/test-evidence.ts,
alongside the classifyTestCoverage/hasLocalTestEvidence/isCodeFile/isTestPath primitives it already
uses). All THREE surfaces now 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, placed 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.

Parity is therefore true by construction, and the tests pin it: 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 CLI test runs against a black-holed API URL, proving no round-trip. 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.

Closes JSONbored#6749
@davion-knight
davion-knight requested a review from JSONbored as a code owner July 17, 2026 07:22
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

⚠️ JUnit XML file not found

The CLI was unable to find any JUnit XML files to upload.
For more help, visit our troubleshooting guide.

@loopover-orb loopover-orb Bot added the gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier. label Jul 17, 2026
@loopover-orb

loopover-orb Bot commented Jul 17, 2026

Copy link
Copy Markdown

Caution

🛑 LoopOver review result - reject/close recommended

Review updated: 2026-07-17 07:28:48 UTC

7 files · 1 AI reviewer · 1 blocker · CI failing · blocked

🛑 Suggested Action - Reject/Close

Review summary
This PR extracts the existing checkTestEvidence classification logic verbatim into a new exported buildTestEvidenceReport in packages/loopover-engine/src/signals/test-evidence.ts and wires it into a new POST /v1/lint/test-evidence route, a new loopover_check_test_evidence stdio tool, and server.ts's checkTestEvidence delegating to it — a reasonable single-source-of-truth design mirroring the existing slop-risk parity pattern. However, the new buildTestEvidenceReport passes input.tests (typed string | undefined) and input.testFiles (typed readonly string[] | undefined) straight into hasLocalTestEvidence, whose signature requires tests?: string[] | undefined and testFiles?: string[] | undefined — a type mismatch on both arguments visible in the same file. This lines up with the CI results: validate, validate-code, and all six validate-tests shards failed on this commit.

Blockers

  • packages/loopover-engine/src/signals/test-evidence.ts:143 — buildTestEvidenceReport calls `hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles })` where `input.tests` is typed `string | undefined` and `input.testFiles` is `readonly string[] | undefined`, but `hasLocalTestEvidence` (same file, unchanged) requires `tests?: string[] | undefined` and a mutable `testFiles?: string[] | undefined` — this is a real type-signature mismatch on newly-added code, not a pre-existing untouched line, and should fail typecheck.
Nits — 4 non-blocking
  • packages/loopover-engine/src/signals/test-evidence.ts:143 — even if the types were reconciled, at runtime a bare string passed where an array is expected happens to work only because `.length` exists on both string and array, which is fragile; prefer wrapping as `tests: input.tests ? [input.tests] : undefined` to make the intent explicit.
  • test/unit/routes-test-evidence.test.ts and mcp-cli-test-evidence-tool.test.ts both import buildTestEvidenceReport directly to assert route/tool output equals the builder's own output — this proves wiring parity but not independent correctness of the builder's classification arms, since both sides call the identical function.
  • Fix the hasLocalTestEvidence call signature in buildTestEvidenceReport (test-evidence.ts:143) to pass an array for `tests`, matching the function's declared type.
  • Consider a small standalone unit test for buildTestEvidenceReport itself (not just via the route/CLI wrappers) asserting exact classification values for each ratio boundary (0.2, 0.4) to catch regressions independent of the two consuming surfaces.

Why this is blocked

  • packages/loopover-engine/src/signals/test-evidence.ts:143 — buildTestEvidenceReport calls `hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles })` where `input.tests` is typed `string | undefined` and `input.testFiles` is `readonly string[] | undefined`, but `hasLocalTestEvidence` (same file, unchanged) requires `tests?: string[] | undefined` and a mutable `testFiles?: string[] | undefined` — this is a real type-signature mismatch on newly-added code, not a pre-existing untouched line, and should fail typecheck.
📋 Copy for AI agents — paste into your coding agent
Fix the following blocker(s) from this PR review:

1. packages/loopover-engine/src/signals/test-evidence.ts:143 — buildTestEvidenceReport calls \`hasLocalTestEvidence\(\{ tests: input.tests, testFiles: input.testFiles \}\)\` where \`input.tests\` is typed \`string \| undefined\` and \`input.testFiles\` is \`readonly string\[\] \| undefined\`, but \`hasLocalTestEvidence\` \(same file, unchanged\) requires \`tests?: string\[\] \| undefined\` and a mutable \`testFiles?: string\[\] \| undefined\` — this is a real type-signature mismatch on newly-added code, not a pre-existing untouched line, and should fail typecheck.

CI checks failing

  • validate
  • validate-tests (3)
  • validate-tests (1)
  • validate-tests (6)
  • validate-tests (4)
  • validate-tests (5)
  • validate-tests (2)
  • validate-code
  • Build UI preview artifact

Decision drivers

  • ❌ Code review — 1 blocker (1 reviewer)
  • ❌ Gate result — Blocking (Repo-configured hard blocker found.)
Context & advisory signals — never blocks the verdict
Signal Result Evidence
Linked issue ✅ Linked #6749
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 178 registered-repo PR(s), 112 merged, 0 issue(s).
Contributor context ✅ Confirmed Gittensor contributor davion-knight; Gittensor profile; 178 PR(s), 0 issue(s).
Improvement ✅ Minor risk: clean · value: minor · LLM: moderate
Linked issue satisfaction

Partially addressed
The REST route POST /v1/lint/test-evidence is added and thoroughly tested against the shared buildTestEvidenceReport builder, satisfying that deliverable well. However, the issue explicitly asks for a 'test-evidence CLI subcommand' mirroring the precedent 'slopRiskCli', and the diff instead only registers loopover_check_test_evidence as an in-process stdio MCP tool (tested via the MCP client proto

Review context
  • Author: davion-knight
  • Role context: outside_contributor
  • Public audience mode: oss maintainer
  • Lane context: Repository is configured for direct PR review.
  • Public profile languages: Rust
  • Official Gittensor activity: 178 PR(s), 0 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (PR #6805)
  • Related work: Titles/paths share 8 meaningful terms. (issue #6744, issue #6748)
  • Related work: Titles/paths share 8 meaningful terms. (issue #6744, issue #6749)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Start here: Review top overlaps.
  • Then work through the remaining 2 steps in the Signals table above.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.
🧪 Chat with LoopOver

Ask 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.

  • @loopover ask <question> answers contribution-quality Q&A with source citations and freshness.
  • @loopover chat <question> answers in natural prose from cached decision-pack facts via local inference (maintainer/collaborator; read-only).
  • A plain-language @loopover mention with a real question is routed to the closest matching read-only command automatically — no exact syntax required.

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.

  • Re-run LoopOver review

@loopover-orb

loopover-orb Bot commented Jul 17, 2026

Copy link
Copy Markdown

LoopOver is closing this pull request on the maintainer's behalf (CI is failing (validate, validate-tests (3), validate-tests (1), validate-tests (6), validate-tests (4), validate-tests (5), validate-tests (2), validate-code, Build UI preview artifact); AI reviewers agree on a likely critical defect: packages/loopover-engine/src/signals/test-evidence.ts:143 — buildTestEvidenceReport calls `hasLocalTestEvidence({ tests: input.tests, testFiles: input.testFiles })` where `input.tests` is typed `string | undefined` and `input.testFiles` is `readonly string[] | undefined`, but `hasLocalTestEvidence` (same file, unchanged) requires `tests?: string[] | undefined` and a mutable `testFiles?: string[] | undefined` — this is a real type-signature mismatch on newly-added code, not a pre-existing untouched line, and should fail typecheck.). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:feature Gittensor-scored feature linked to a feature issue — scores a 0.25x multiplier.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

REST + CLI mirror for loopover_check_test_evidence

1 participant