From 276d2855b64e4684bb709044038a993b04551aa3 Mon Sep 17 00:00:00 2001 From: Laith Al-Saadoon <9553966+theagenticguy@users.noreply.github.com> Date: Fri, 5 Jun 2026 16:40:21 -0500 Subject: [PATCH] feat(mcp): surface ingested test-coverage on context and impact MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The coverage overlay phase already annotates File and callable nodes with a coveragePercent ratio (consumed today only by verdict's complex_and_untested signal). This reads that data out on the two high-frequency exploration tools. context: add an optional `coverage` block { percent, covered, source } to a resolved target. Reads the per-symbol ratio carried on the node (callables get one from the coverage phase); a symbol with no own ratio inherits its enclosing File node's ratio (source="file"). The block is OMITTED entirely when no report was ingested — absent coverage is UNKNOWN, never rendered as 0%. impact: add an optional `untestedBlastRadius` summary over the depth-1 dependents, splitting them into untested (known coverage below the 0.5 threshold), tested, and unknownCoverage (no coverage ingested). Dependents with no ingested coverage land in unknownCoverage, never in untested, so a repo that never ran coverage is not misreported as fully untested. The field is omitted when there are no direct dependents. Both thresholds match verdict's complex_and_untested escalation (0.5) so the three surfaces agree on what "untested" means. ToolResult shape (structuredContent + text) is preserved; the new fields are additive and optional. Tool descriptions document the new fields and the absent-vs-zero contract. Tests: context asserts coverage present (per-symbol + file-inherited), thin coverage flagged not-covered, and field omitted (no false 0%) when absent. impact asserts the tested/untested/unknown split, that absent coverage is unknown not untested, and that the field is omitted with zero dependents. --- packages/mcp/src/tool-handlers.test.ts | 151 ++++++++++++++++++++++ packages/mcp/src/tools/context.test.ts | 115 +++++++++++++++++ packages/mcp/src/tools/context.ts | 100 ++++++++++++++- packages/mcp/src/tools/impact.ts | 167 ++++++++++++++++++++++++- 4 files changed, 530 insertions(+), 3 deletions(-) diff --git a/packages/mcp/src/tool-handlers.test.ts b/packages/mcp/src/tool-handlers.test.ts index d3e12cd2..e6029b57 100644 --- a/packages/mcp/src/tool-handlers.test.ts +++ b/packages/mcp/src/tool-handlers.test.ts @@ -675,6 +675,157 @@ test("impact surfaces cochanges for the target's file as a side section", async ); }); +test("impact: untestedBlastRadius classifies direct dependents by coverage", async () => { + await withTestHarness( + { + nodes: [ + { id: "Function:src/foo.ts:foo", name: "foo", kind: "Function", file_path: "src/foo.ts" }, + // Two direct dependents (depth-1, upstream): one well-covered, one thin. + { + id: "Function:src/well.ts:well", + name: "well", + kind: "Function", + file_path: "src/well.ts", + coveragePercent: 0.9, + }, + { + id: "Function:src/thin.ts:thin", + name: "thin", + kind: "Function", + file_path: "src/thin.ts", + coveragePercent: 0.1, + }, + ], + relations: [ + { + id: "E:1", + from_id: "Function:src/well.ts:well", + to_id: "Function:src/foo.ts:foo", + type: "CALLS", + confidence: 0.9, + }, + { + id: "E:2", + from_id: "Function:src/thin.ts:thin", + to_id: "Function:src/foo.ts:foo", + type: "CALLS", + confidence: 0.9, + }, + ], + }, + async (ctx, server) => { + registerImpactTool(server, ctx); + const handler = getHandler(server, "impact"); + const result = await handler( + { target: "Function:src/foo.ts:foo", direction: "upstream", repo: "fakerepo" }, + {}, + ); + const sc = result.structuredContent as { + untestedBlastRadius?: { + threshold: number; + directCount: number; + testedCount: number; + untestedCount: number; + unknownCount: number; + untested: Array<{ id: string; coveragePercent: number | null }>; + unknownCoverage: Array<{ id: string }>; + }; + }; + assert.ok( + sc.untestedBlastRadius, + "untestedBlastRadius present when there are direct dependents", + ); + const ubr = sc.untestedBlastRadius; + assert.equal(ubr?.directCount, 2); + assert.equal(ubr?.testedCount, 1, "well-covered dependent counted as tested"); + assert.equal(ubr?.untestedCount, 1, "thin dependent counted as untested"); + assert.equal(ubr?.unknownCount, 0); + assert.equal(ubr?.untested[0]?.id, "Function:src/thin.ts:thin"); + assert.equal(ubr?.untested[0]?.coveragePercent, 0.1); + const first = result.content[0]; + assert.ok(first && first.type === "text"); + assert.match(first.text, /Untested blast radius/); + }, + ); +}); + +test("impact: dependents with no ingested coverage land in unknownCoverage, never untested", async () => { + await withTestHarness( + { + nodes: [ + { id: "Function:src/foo.ts:foo", name: "foo", kind: "Function", file_path: "src/foo.ts" }, + // Direct dependent with NO coveragePercent and NO covered File node → + // must be UNKNOWN, not a false 0% / untested. + { + id: "Function:src/dep.ts:dep", + name: "dep", + kind: "Function", + file_path: "src/dep.ts", + }, + ], + relations: [ + { + id: "E:1", + from_id: "Function:src/dep.ts:dep", + to_id: "Function:src/foo.ts:foo", + type: "CALLS", + confidence: 0.9, + }, + ], + }, + async (ctx, server) => { + registerImpactTool(server, ctx); + const handler = getHandler(server, "impact"); + const result = await handler( + { target: "Function:src/foo.ts:foo", direction: "upstream", repo: "fakerepo" }, + {}, + ); + const sc = result.structuredContent as { + untestedBlastRadius?: { + untestedCount: number; + unknownCount: number; + untested: Array<{ id: string }>; + unknownCoverage: Array<{ id: string; coveragePercent: number | null }>; + }; + }; + assert.ok(sc.untestedBlastRadius); + assert.equal(sc.untestedBlastRadius?.untestedCount, 0, "no coverage ≠ untested"); + assert.equal(sc.untestedBlastRadius?.unknownCount, 1); + assert.equal(sc.untestedBlastRadius?.unknownCoverage[0]?.id, "Function:src/dep.ts:dep"); + assert.equal( + sc.untestedBlastRadius?.unknownCoverage[0]?.coveragePercent, + null, + "unknown coverage carries null, not 0", + ); + }, + ); +}); + +test("impact: untestedBlastRadius omitted when there are no direct dependents", async () => { + await withTestHarness( + { + nodes: [ + { id: "Function:src/foo.ts:foo", name: "foo", kind: "Function", file_path: "src/foo.ts" }, + ], + relations: [], + }, + async (ctx, server) => { + registerImpactTool(server, ctx); + const handler = getHandler(server, "impact"); + const result = await handler( + { target: "Function:src/foo.ts:foo", direction: "upstream", repo: "fakerepo" }, + {}, + ); + const sc = result.structuredContent as { untestedBlastRadius?: unknown }; + assert.equal( + sc.untestedBlastRadius, + undefined, + "no direct dependents → no untestedBlastRadius noise", + ); + }, + ); +}); + test("context: confidenceBreakdown tallies LSP-confirmed vs heuristic vs demoted edges", async () => { await withTestHarness( { diff --git a/packages/mcp/src/tools/context.test.ts b/packages/mcp/src/tools/context.test.ts index 5fe4b41d..e1d69ace 100644 --- a/packages/mcp/src/tools/context.test.ts +++ b/packages/mcp/src/tools/context.test.ts @@ -247,6 +247,121 @@ test("context: include_content attaches source (capped at 2000 chars)", async () ); }); +test("context: surfaces per-symbol coverage when the overlay ingested a ratio", async () => { + await withHarness( + { + nodes: [ + // Callable carries a per-symbol coveragePercent from the coverage phase. + { + id: "F:tested", + name: "tested", + kind: "Function", + file_path: "src/tested.ts", + coveragePercent: 0.92, + }, + ], + relations: [], + }, + async (ctx, server) => { + registerContextTool(server, ctx); + const handler = getToolHandler(server, "context"); + const result = await handler({ uid: "F:tested", repo: "fakerepo" }, {}); + const sc = result.structuredContent as { + coverage?: { percent: number; covered: boolean; source: string }; + }; + assert.ok(sc.coverage, "coverage block present when a ratio was ingested"); + assert.equal(sc.coverage?.percent, 0.92); + assert.equal(sc.coverage?.covered, true, "0.92 ≥ 0.5 threshold → covered"); + assert.equal(sc.coverage?.source, "symbol"); + const first = result.content[0]; + assert.ok(first && first.type === "text"); + assert.match(first.text, /Coverage: 92\.0% \(covered, from symbol\)/); + }, + ); +}); + +test("context: thin per-symbol coverage is reported as not covered", async () => { + await withHarness( + { + nodes: [ + { + id: "F:thin", + name: "thin", + kind: "Function", + file_path: "src/thin.ts", + coveragePercent: 0.1, + }, + ], + relations: [], + }, + async (ctx, server) => { + registerContextTool(server, ctx); + const handler = getToolHandler(server, "context"); + const result = await handler({ uid: "F:thin", repo: "fakerepo" }, {}); + const sc = result.structuredContent as { + coverage?: { percent: number; covered: boolean }; + }; + assert.ok(sc.coverage); + assert.equal(sc.coverage?.percent, 0.1); + assert.equal(sc.coverage?.covered, false, "0.1 < 0.5 threshold → not covered"); + }, + ); +}); + +test("context: a symbol with no coverage inherits its enclosing File ratio", async () => { + await withHarness( + { + nodes: [ + // The symbol carries NO coveragePercent; its enclosing File node does. + { id: "F:nocov", name: "nocov", kind: "Function", file_path: "src/mixed.ts" }, + { + id: "File:mixed", + name: "mixed.ts", + kind: "File", + file_path: "src/mixed.ts", + coveragePercent: 0.7, + }, + ], + relations: [], + }, + async (ctx, server) => { + registerContextTool(server, ctx); + const handler = getToolHandler(server, "context"); + const result = await handler({ uid: "F:nocov", repo: "fakerepo" }, {}); + const sc = result.structuredContent as { + coverage?: { percent: number; covered: boolean; source: string }; + }; + assert.ok(sc.coverage, "coverage inherited from the enclosing File node"); + assert.equal(sc.coverage?.percent, 0.7); + assert.equal(sc.coverage?.source, "file"); + }, + ); +}); + +test("context: coverage field is OMITTED when no report was ingested (never a false 0%)", async () => { + await withHarness( + { + nodes: [ + // No coveragePercent anywhere on the symbol or its file. + { id: "F:uncov", name: "uncov", kind: "Function", file_path: "src/uncov.ts" }, + ], + relations: [], + }, + async (ctx, server) => { + registerContextTool(server, ctx); + const handler = getToolHandler(server, "context"); + const result = await handler({ uid: "F:uncov", repo: "fakerepo" }, {}); + const sc = result.structuredContent as { coverage?: unknown }; + assert.equal(sc.coverage, undefined, "absent coverage → field omitted, not 0%"); + const first = result.content[0]; + assert.ok(first && first.type === "text"); + assert.match(first.text, /Coverage: unknown/); + // Critically, the text must NOT imply 0% when coverage was never ingested. + assert.doesNotMatch(first.text, /Coverage: 0\.0%/); + }, + ); +}); + test("context: categorises incoming + outgoing edges by edge type", async () => { await withHarness( { diff --git a/packages/mcp/src/tools/context.ts b/packages/mcp/src/tools/context.ts index 6a52d8d7..d0309bcf 100644 --- a/packages/mcp/src/tools/context.ts +++ b/packages/mcp/src/tools/context.ts @@ -19,6 +19,11 @@ * - `operations` — OpenAPI `Operation` nodes linked to a Route target via * `HANDLES_ROUTE` (cross-stack trace from the OpenAPI phase). * - `confidenceBreakdown` — provenance tally over every edge surfaced. + * - `coverage` (optional) — line-level test coverage for the target, + * read from the `coverage` overlay phase. Present only when a coverage + * report was ingested for the target (or its enclosing file); ABSENT + * coverage is treated as UNKNOWN and the field is omitted entirely so a + * caller never mistakes "not ingested" for "0% covered". * - `content` (optional) — the target's indexed source, capped at * {@link CONTENT_CHAR_CAP} characters. Only returned when * `include_content` is true. @@ -50,6 +55,13 @@ import { /** Upper bound on the `content` field size when `include_content` is true. */ const CONTENT_CHAR_CAP = 2000; +/** + * Coverage ratio below which a symbol is flagged as thinly tested. Matches the + * `verdict` tool's `complex_and_untested` escalation threshold so the two + * surfaces agree on what "untested" means. + */ +const COVERAGE_THIN_THRESHOLD = 0.5; + /** * Relation types surfaced in the categorised `incoming` / `outgoing` * buckets. PROCESS_STEP / HANDLES_ROUTE / CONTAINS live in their own @@ -144,6 +156,21 @@ interface TargetLocation { readonly endLine: number | null; } +/** + * Coverage block attached to a resolved target when the `coverage` overlay + * phase ingested a report for it. `percent` is the line-level ratio in + * [0, 1]; `covered` is the human-friendly verdict against + * {@link COVERAGE_THIN_THRESHOLD}; `source` records whether the ratio came + * from the target symbol itself or was inherited from its enclosing file. + * When coverage was never ingested the whole block is omitted — absent + * coverage is UNKNOWN, never 0%. + */ +interface TargetCoverage { + readonly percent: number; + readonly covered: boolean; + readonly source: "symbol" | "file"; +} + /** The seven canonical category buckets plus the two override-kind buckets. */ type CategoryBuckets = { calls: NodeRow[]; @@ -248,6 +275,7 @@ export async function runContext(ctx: ToolContext, args: ContextArgs): Promise 0) { @@ -346,6 +382,10 @@ export async function runContext(ctx: ToolContext, args: ContextArgs): Promise 1) return null; + return raw; +} + +/** + * Resolve the coverage block for a target. Prefers the ratio carried on the + * resolved node itself (callables get a per-symbol ratio from the coverage + * phase); for File targets that ratio already IS the file ratio. Otherwise + * falls back to the enclosing File node's `coveragePercent` so a symbol + * inside a covered file still reports coverage. Returns `undefined` when no + * coverage was ingested anywhere on the path — the caller omits the field. + */ +async function fetchTargetCoverage( + graph: IGraphStore, + target: NodeRow, + ownCoverage: number | null, +): Promise { + if (ownCoverage !== null) { + return { + percent: ownCoverage, + covered: ownCoverage >= COVERAGE_THIN_THRESHOLD, + source: target.kind === "File" ? "file" : "symbol", + }; + } + // Symbol carried no coverage — inherit from its enclosing File node when one + // exists with an ingested ratio. File targets with no own ratio fall through + // to UNKNOWN (we don't re-query the same node). + if (target.kind === "File" || target.filePath.length === 0) return undefined; + const fileNodes = await graph.listNodesByKind("File", { filePath: target.filePath }); + for (const node of fileNodes) { + const fileCov = coverageOrNull(getProp(node, "coveragePercent")); + if (fileCov !== null) { + return { + percent: fileCov, + covered: fileCov >= COVERAGE_THIN_THRESHOLD, + source: "file", + }; + } + } + return undefined; +} + function capContent(raw: string | null): string | undefined { if (raw === null) return undefined; if (raw.length <= CONTENT_CHAR_CAP) return raw; diff --git a/packages/mcp/src/tools/impact.ts b/packages/mcp/src/tools/impact.ts index 29e99962..63097d13 100644 --- a/packages/mcp/src/tools/impact.ts +++ b/packages/mcp/src/tools/impact.ts @@ -18,7 +18,8 @@ import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import type { AffectedModule, AffectedProcess, ImpactDepthBucket } from "@opencodehub/analysis"; -import type { ITemporalStore } from "@opencodehub/storage"; +import type { GraphNode } from "@opencodehub/core-types"; +import type { IGraphStore, ITemporalStore } from "@opencodehub/storage"; import { z } from "zod"; import { callRunImpact } from "../analysis-bridge.js"; import { toolError, toolErrorFromUnknown } from "../error-envelope.js"; @@ -41,6 +42,40 @@ interface ImpactCochangePartner { readonly lastCocommitAt: string; } +/** + * Coverage ratio below which a dependent is flagged untested. Matches the + * `verdict` tool's `complex_and_untested` escalation threshold and the + * `context` tool's coverage verdict so all three surfaces agree. + */ +const COVERAGE_THIN_THRESHOLD = 0.5; + +/** A direct dependent annotated with its (optional) coverage ratio. */ +interface CoveredDependent { + readonly id: string; + readonly name: string; + readonly kind: string; + readonly filePath: string; + /** Line-level coverage ratio in [0, 1], or null when none was ingested. */ + readonly coveragePercent: number | null; +} + +/** + * Untested-blast-radius summary over the depth-1 dependents. `untested` + * lists dependents whose KNOWN coverage is below the threshold; `unknown` + * lists dependents with no ingested coverage at all. The split is the whole + * point: a depth-1 node with no coverage report is UNKNOWN, never counted as + * untested, so a repo that simply never ran coverage is not maligned. + */ +interface UntestedBlastRadius { + readonly threshold: number; + readonly directCount: number; + readonly testedCount: number; + readonly untestedCount: number; + readonly unknownCount: number; + readonly untested: readonly CoveredDependent[]; + readonly unknownCoverage: readonly CoveredDependent[]; +} + const ImpactInput = { target: z .string() @@ -161,6 +196,9 @@ export async function runImpact(ctx: ToolContext, args: ImpactArgs): Promise 0) { + lines.push( + `Untested blast radius (direct dependents, coverage<${(COVERAGE_THIN_THRESHOLD * 100).toFixed(0)}%): ` + + `${untestedBlastRadius.untestedCount} untested, ` + + `${untestedBlastRadius.testedCount} tested, ` + + `${untestedBlastRadius.unknownCount} unknown coverage`, + ); + for (const n of untestedBlastRadius.untested.slice(0, 20)) { + const pct = n.coveragePercent === null ? "?" : `${(n.coveragePercent * 100).toFixed(1)}%`; + lines.push(` ⚠ ${n.name} [${n.kind}] — ${n.filePath || "(no file)"} (coverage=${pct})`); + } + if (untestedBlastRadius.untested.length > 20) { + lines.push(` … ${untestedBlastRadius.untested.length - 20} more`); + } + } if (cochanges.length > 0) { lines.push( `Files often edited together with this one (by lift) — git history, NOT call dependencies (${cochanges.length}):`, @@ -224,6 +277,11 @@ export async function runImpact(ctx: ToolContext, args: ImpactArgs): Promise 0) { + next.push( + `${untestedBlastRadius.untestedCount} direct dependent(s) are thinly tested (coverage<${(COVERAGE_THIN_THRESHOLD * 100).toFixed(0)}%) — add tests there before changing the target's contract`, + ); + } return withNextSteps( lines.join("\n"), @@ -240,6 +298,12 @@ export async function runImpact(ctx: ToolContext, args: ImpactArgs): Promise)["coveragePercent"]; + if (typeof raw !== "number" || !Number.isFinite(raw)) return null; + if (raw < 0 || raw > 1) return null; + return raw; +} + +/** + * Classify the depth-1 dependents by test coverage so the caller can see the + * "untested blast radius" — which directly-impacted symbols are NOT backed by + * tests and so will break silently. + * + * Coverage is read per-symbol first (callables carry a `coveragePercent` from + * the coverage phase); dependents that lack a per-symbol ratio inherit their + * enclosing File node's ratio. A dependent with NO ingested coverage anywhere + * is reported under `unknownCoverage` — never under `untested` — so a repo + * that never ran coverage is not misreported as fully untested. + * + * Returns `undefined` when there are no direct dependents at all; the caller + * omits the field so a zero-blast-radius change carries no coverage noise. + */ +async function fetchUntestedBlastRadius( + graph: IGraphStore, + direct: readonly { + readonly id: string; + readonly name: string; + readonly kind: string; + readonly filePath: string; + }[], +): Promise { + if (direct.length === 0) return undefined; + + // Per-symbol coverage: one bulk lookup over the direct dependent ids. + const ids = Array.from(new Set(direct.map((d) => d.id))).filter((id) => id.length > 0); + const symbolCov = new Map(); + if (ids.length > 0) { + const nodes = await graph.listNodes({ ids }); + for (const n of nodes) symbolCov.set(n.id, coverageOf(n)); + } + + // File-level fallback: gather the file paths of dependents whose own node + // carried no coverage, then look up the File node for each in one sweep. + const filesToProbe = new Set(); + for (const d of direct) { + const own = symbolCov.get(d.id) ?? null; + if (own === null && d.kind !== "File" && d.filePath.length > 0) { + filesToProbe.add(d.filePath); + } + } + const fileCov = new Map(); + await Promise.all( + [...filesToProbe].map(async (filePath) => { + const fileNodes = await graph.listNodesByKind("File", { filePath }); + let ratio: number | null = null; + for (const node of fileNodes) { + const c = coverageOf(node); + if (c !== null) { + ratio = c; + break; + } + } + fileCov.set(filePath, ratio); + }), + ); + + const untested: CoveredDependent[] = []; + const unknownCoverage: CoveredDependent[] = []; + let testedCount = 0; + for (const d of direct) { + const own = symbolCov.get(d.id) ?? null; + const percent = own ?? (d.kind !== "File" ? (fileCov.get(d.filePath) ?? null) : null); + const entry: CoveredDependent = { + id: d.id, + name: d.name, + kind: d.kind, + filePath: d.filePath, + coveragePercent: percent, + }; + if (percent === null) { + unknownCoverage.push(entry); + } else if (percent < COVERAGE_THIN_THRESHOLD) { + untested.push(entry); + } else { + testedCount += 1; + } + } + + return { + threshold: COVERAGE_THIN_THRESHOLD, + directCount: direct.length, + testedCount, + untestedCount: untested.length, + unknownCount: unknownCoverage.length, + untested, + unknownCoverage, + }; +}