From f65a2218f5cf875c4f20b1e7e11fbccd93c4a522 Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:44:35 -0700 Subject: [PATCH] feat(mcp): add gittensory_check_improvement_potential pre-submit tool Adds a supply-side pre-submit MCP tool that mirrors gittensory_check_slop_risk but for the positive improvement axis: it calls buildStructuralImprovementAssessment (src/signals/improvement.ts, already shipped) directly from the MCP layer rather than re-deriving the scoring math, following the same metadata-only, no-source-upload contract as its sibling. complexityDeltas/duplicationDeltas are optional already-derived structured deltas the calling agent supplies; the tool never reads file content or diffs itself, and degrades cleanly to insufficient-signal when every input is omitted. Unlike checkSlopRisk, the raw score is not blunted here: improvementScore carries no gate/blocker power, so there is nothing to protect from reverse-engineering, and the whole point of the signal is to let a contributor see how close a planned change is to the next band. The LLM-tier judgment (ModelReview.valueAssessment) is intentionally out of scope for this tool -- it depends on a live AI-review call against an already-opened PR, which does not exist at pre-submit time. Also updates reference.md section 5's tool list (new entry plus a stale doc fix: the existing check_slop_risk entry still described a raw slopRisk field the handler stopped returning under the #mcp-slop-blunt change). Part of #4737. --- .../contributing-to-gittensory/reference.md | 14 ++- src/mcp/server.ts | 83 +++++++++++++++++ .../mcp-check-improvement-potential.test.ts | 89 +++++++++++++++++++ 3 files changed, 182 insertions(+), 4 deletions(-) create mode 100644 test/unit/mcp-check-improvement-potential.test.ts diff --git a/.claude/skills/contributing-to-gittensory/reference.md b/.claude/skills/contributing-to-gittensory/reference.md index f494863f6a..d04625e1ef 100644 --- a/.claude/skills/contributing-to-gittensory/reference.md +++ b/.claude/skills/contributing-to-gittensory/reference.md @@ -141,12 +141,18 @@ All tools are metadata-only (no source upload). Run in this order: 2. `gittensory_validate_linked_issue` — `{owner, repo, issueNumber, plannedChange}` → is the issue open, valid, single-owner, solvable by this PR. 3. `gittensory_check_slop_risk` — `{changedFiles[{path,additions,deletions}], description, tests, - testFiles}` → slopRisk 0–100 + band + findings. -4. `gittensory_lint_pr_text` — `{commitMessages[], prBody, linkedIssue}` → verdict + testFiles}` → band + findings. +4. `gittensory_check_improvement_potential` — `{changedFiles?[{path,additions,deletions}], tests?, + testFiles?, patchCoverageDeltaPercent?, complexityDeltas?[{file,line,name,before,after,delta}], + duplicationDeltas?[{file,line,duplicateOfLine,lines}]}` → improvementScore + band + (insufficient-signal/none/minor/moderate/significant) + findings. The positive-axis mirror of + `gittensory_check_slop_risk` — deterministic tier only (no LLM judgment); complexityDeltas/ + duplicationDeltas are optional precomputed deltas the calling agent supplies, never raw source. +5. `gittensory_lint_pr_text` — `{commitMessages[], prBody, linkedIssue}` → verdict strong/adequate/weak + specific fixes. -5. `gittensory_validate_config` — `{content, source?}` → normalized manifest fields, +6. `gittensory_validate_config` — `{content, source?}` → normalized manifest fields, warnings, and ok/warn/error status. -6. `gittensory_predict_gate` — `{login, owner, repo, title, body, labels, linkedIssues}` → predicted +7. `gittensory_predict_gate` — `{login, owner, repo, title, body, labels, linkedIssues}` → predicted conclusion + blockers + warnings + readiness score. (Auth'd extras: `gittensory_preflight_pr` / `…_local_diff` for lane fit + collision + queue health; diff --git a/src/mcp/server.ts b/src/mcp/server.ts index e9c3c0df35..0b62ba8050 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -154,6 +154,7 @@ import { MAX_FOCUS_MANIFEST_BYTES } from "../signals/focus-manifest"; import { loadPublicRepoFocusManifest, loadRepoFocusManifest } from "../signals/focus-manifest-loader"; import { buildPredictedGateVerdict, type PredictedGateVerdict } from "../rules/predicted-gate"; import { buildIssueSlopAssessment, buildSlopAssessment } from "../signals/slop"; +import { buildStructuralImprovementAssessment } from "../signals/improvement"; import { buildBoundaryTestGenerationFinding, buildBoundaryTestGenerationSpec } from "../signals/boundary-test-generation"; import { buildRepoDataQuality } from "../signals/data-quality"; import { PREFLIGHT_LIMITS } from "../signals/preflight-limits"; @@ -918,6 +919,62 @@ const checkSlopRiskOutputSchema = { rubric: z.string().optional(), }; +// Deterministic structural-improvement counterpart to checkSlopRiskShape (#4746, sub-issue I of epic #4737): +// the positive-axis mirror of checkSlopRisk, same pure local-metadata contract. changedFiles/tests/testFiles +// are reused verbatim (same shape as checkSlopRiskShape) so the two signals never disagree about what counts +// as test evidence. complexityDeltas/duplicationDeltas mirror ComplexityDeltaLike/DuplicationDeltaLike +// (src/signals/improvement.ts) as already-derived structured deltas — the calling agent computes them from +// its own local working tree (real before/after content, no reconstructOldContent trick needed) and supplies +// them here; this tool never reads file content or diffs itself. Every field is optional: +// buildStructuralImprovementAssessment degrades cleanly to "insufficient-signal" when nothing is supplied +// (see its own tests), so there is no synthetic "at least one field required" check to duplicate here. No +// auth required — same choice as checkSlopRisk: a pure function over caller-supplied structured data with no +// owner/repo/login to scope, and improvementScore carries no gate/blocker power (advisory-only; see +// improvement.ts's header comment), so there is nothing to gate. +const checkImprovementPotentialShape = { + changedFiles: z + .array(z.object({ path: z.string().min(1).max(400), additions: z.number().int().min(0).optional(), deletions: z.number().int().min(0).optional() })) + .max(2000) + .optional(), + tests: z.array(z.string().max(400)).max(2000).optional(), + testFiles: z.array(z.string().max(400)).max(2000).optional(), + patchCoverageDeltaPercent: z.number().optional(), + complexityDeltas: z + .array( + z.object({ + file: z.string().min(1).max(400), + line: z.number().int().min(1), + name: z.string().min(1).max(400), + before: z.number().int().min(0), + after: z.number().int().min(0), + delta: z.number().int(), + }), + ) + .max(2000) + .optional(), + duplicationDeltas: z + .array( + z.object({ + file: z.string().min(1).max(400), + line: z.number().int().min(1), + duplicateOfLine: z.number().int().min(1), + lines: z.number().int().min(1), + }), + ) + .max(2000) + .optional(), +}; + +// Unlike checkSlopRiskOutputSchema, the numeric score is NOT blunted: improvementScore has no gate/blocker +// power (unlike slopRisk, which the blunting explicitly protects from reverse-engineering an evasion of a +// block — #mcp-slop-blunt), and the whole point of a supply-side pre-submit value signal is to let a miner +// see how close their planned change is to the next band, so hiding the number would defeat the tool. +const checkImprovementPotentialOutputSchema = { + improvementScore: z.number().optional(), + band: z.enum(["insufficient-signal", "none", "minor", "moderate", "significant"]).optional(), + findings: z.unknown().optional(), +}; + // Coverage-gap self-check (#2235): pure local-metadata, like checkSlopRisk — the agent supplies its changed // paths (plus any test paths) and asks whether the change carries enough test evidence, no source uploaded. const checkTestEvidenceShape = { @@ -1623,6 +1680,17 @@ export class GittensoryMcp { async (input) => this.toolResult(await this.checkSlopRisk(input)), ); + server.registerTool( + "gittensory_check_improvement_potential", + { + description: + "Assess the deterministic structural-improvement potential of a planned change from local diff metadata (paths + line counts) plus optional precomputed complexity/duplication deltas and a patch-coverage delta — an agent-native, source-free positive-signal self-check mirroring gittensory_check_slop_risk. Returns the score, band (insufficient-signal/none/minor/moderate/significant), and actionable findings. Deterministic tier only (no LLM judgment); no repo data needed.", + inputSchema: checkImprovementPotentialShape, + outputSchema: checkImprovementPotentialOutputSchema, + }, + async (input) => this.toolResult(await this.checkImprovementPotential(input)), + ); + server.registerTool( "gittensory_check_test_evidence", { @@ -2883,6 +2951,21 @@ export class GittensoryMcp { }; } + private async checkImprovementPotential( + input: z.infer>, + ): Promise { + await this.enforceToolRateLimit("gittensory_check_improvement_potential"); + const assessment = buildStructuralImprovementAssessment(input); + return { + summary: `Improvement potential: ${assessment.band}.`, + data: { + improvementScore: assessment.improvementScore, + band: assessment.band, + findings: assessment.findings, + } as unknown as Record, + }; + } + private async checkTestEvidence(input: z.infer>): Promise { await this.enforceToolRateLimit("gittensory_check_test_evidence"); const allPaths = [...input.changedPaths, ...(input.testFiles ?? [])]; diff --git a/test/unit/mcp-check-improvement-potential.test.ts b/test/unit/mcp-check-improvement-potential.test.ts new file mode 100644 index 0000000000..b0e3ffa4b3 --- /dev/null +++ b/test/unit/mcp-check-improvement-potential.test.ts @@ -0,0 +1,89 @@ +import { Client } from "@modelcontextprotocol/sdk/client/index.js"; +import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js"; +import { describe, expect, it } from "vitest"; +import { GittensoryMcp } from "../../src/mcp/server"; +import { createTestEnv } from "../helpers/d1"; + +async function connect() { + const server = new GittensoryMcp(createTestEnv()).createServer(); + const [clientTransport, serverTransport] = InMemoryTransport.createLinkedPair(); + await server.connect(serverTransport); + const client = new Client({ name: "gittensory-improvement-potential-test", version: "0.1.0" }, { capabilities: {} }); + await client.connect(clientTransport); + return client; +} + +describe("MCP gittensory_check_improvement_potential (#4746)", () => { + it("is registered with a non-empty description and an outputSchema", async () => { + const client = await connect(); + const { tools } = await client.listTools(); + const tool = tools.find((t) => t.name === "gittensory_check_improvement_potential"); + expect(tool).toBeDefined(); + expect(tool?.description?.length ?? 0).toBeGreaterThan(0); + expect(tool?.outputSchema).toBeDefined(); + expect(tool?.outputSchema?.type).toBe("object"); + }); + + it("degrades to insufficient-signal when every input is omitted", async () => { + const client = await connect(); + const result = await client.callTool({ name: "gittensory_check_improvement_potential", arguments: {} }); + expect(result.isError).toBeFalsy(); + const data = result.structuredContent as { improvementScore: number; band: string; findings: unknown[] }; + expect(data).toEqual({ improvementScore: 0, band: "insufficient-signal", findings: [] }); + }); + + it("still works from just changedFiles/tests/testFiles when complexityDeltas/duplicationDeltas are omitted", async () => { + const client = await connect(); + const result = await client.callTool({ + name: "gittensory_check_improvement_potential", + arguments: { + changedFiles: [ + { path: "src/widget.ts", additions: 20, deletions: 5 }, + { path: "test/unit/widget.test.ts", additions: 30, deletions: 0 }, + ], + }, + }); + expect(result.isError).toBeFalsy(); + const data = result.structuredContent as { improvementScore: number; band: string; findings: Array<{ code: string }> }; + expect(data.band).toBe("minor"); + expect(data.findings.map((f) => f.code)).toEqual(["added_test_evidence"]); + // complexityDeltas/duplicationDeltas were never supplied, yet this still produced a real (non-insufficient) band. + expect(data.improvementScore).toBeGreaterThan(0); + }); + + it("reaches `significant` when both structural deltas are supplied, and does NOT blunt improvementScore", async () => { + const client = await connect(); + const result = await client.callTool({ + name: "gittensory_check_improvement_potential", + arguments: { + complexityDeltas: [{ file: "src/a.ts", line: 10, name: "foo", before: 12, after: 4, delta: -8 }], + duplicationDeltas: [{ file: "src/b.ts", line: 5, duplicateOfLine: 55, lines: 9 }], + }, + }); + expect(result.isError).toBeFalsy(); + const data = result.structuredContent as { improvementScore: number; band: string; findings: Array<{ code: string }> }; + expect(data.band).toBe("significant"); + // Unlike gittensory_check_slop_risk (blunted by design, #mcp-slop-blunt), the raw score IS returned here — + // improvementScore has no gate/blocker power, so there is nothing to protect from reverse-engineering. + expect(data).toHaveProperty("improvementScore"); + expect(data.improvementScore).toBe(70); + expect(data.findings.map((f) => f.code).sort()).toEqual(["reduced_complexity", "resolved_duplication"]); + expect(JSON.stringify(data)).not.toMatch(/wallet|hotkey|coldkey|mnemonic|reward|payout|trust score/i); + }); + + it("combines a patch-coverage delta with duplication deltas into one aggregate score/band", async () => { + const client = await connect(); + const result = await client.callTool({ + name: "gittensory_check_improvement_potential", + arguments: { + patchCoverageDeltaPercent: 8, + duplicationDeltas: [{ file: "src/c.ts", line: 3, duplicateOfLine: 30, lines: 6 }], + }, + }); + expect(result.isError).toBeFalsy(); + const data = result.structuredContent as { improvementScore: number; band: string; findings: Array<{ code: string }> }; + expect(data.improvementScore).toBe(55); + expect(data.band).toBe("moderate"); + expect(data.findings.map((f) => f.code).sort()).toEqual(["increased_patch_coverage", "resolved_duplication"]); + }); +});