This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 3cefc5c3431efe0e7117f976fe387b2bc032344a Author: Stephen Mallette <[email protected]> AuthorDate: Fri Jul 3 11:03:12 2026 -0400 tinker-review: edge confidence tags + resilient graph population Tag every graph edge EXTRACTED/INFERRED/AMBIGUOUS, with an auditConfidence command and a Signal Confidence report panel so inferred/guessed links are visible to the reviewer. Stop dropping graph context: materialize external-callee and deleted/unparsed File stubs so calls/tests/modifies edges no longer silently vanish, and report the true vertex/edge totals. Fix a duplicate PR-discussion vertex that doubled every PR-sourced edge, skip PR-deleted files during extraction (was crashing), and exit Phase 1 cleanly so completion is detectable. Assisted-by: Claude Code:claude-opus-4-8 --- .skills/tinker-review/SKILL.md | 15 +++- .skills/tinker-review/references/schema.md | 33 ++++++++ .skills/tinker-review/scripts/enrichment/api.js | 19 +++-- .skills/tinker-review/scripts/enrichment/cli.js | 18 +++-- .../scripts/extraction/tree-sitter.js | 10 ++- .skills/tinker-review/scripts/graph/confidence.js | 85 +++++++++++++++++++ .../scripts/graph/populate-discussions.js | 34 +++++--- .skills/tinker-review/scripts/graph/populate.js | 94 +++++++++++++++++++++- .../scripts/patterns/confidence-audit.js | 94 ++++++++++++++++++++++ .skills/tinker-review/scripts/renderer/render.js | 46 ++++++++++- .skills/tinker-review/scripts/review.js | 19 ++++- 11 files changed, 435 insertions(+), 32 deletions(-) diff --git a/.skills/tinker-review/SKILL.md b/.skills/tinker-review/SKILL.md index 084785509d..30f028258c 100644 --- a/.skills/tinker-review/SKILL.md +++ b/.skills/tinker-review/SKILL.md @@ -80,13 +80,20 @@ node scripts/enrichment/cli.js listFunctions --workDir /tmp/pr-review-<pr> --cha node scripts/enrichment/cli.js listTypes --workDir /tmp/pr-review-<pr> --kind class node scripts/enrichment/cli.js getCallsFrom --workDir /tmp/pr-review-<pr> --function <name> --file <path> node scripts/enrichment/cli.js getCanonicalSteps --workDir /tmp/pr-review-<pr> +node scripts/enrichment/cli.js auditConfidence --workDir /tmp/pr-review-<pr> ``` -**Write commands:** +`auditConfidence` returns the edge confidence distribution and the list of +AMBIGUOUS edges. Re-run it after enrichment to refresh the audit with the edges +you added, then reflect any remaining AMBIGUOUS links in `openQuestions`. + +**Write commands** (edges you create default to `INFERRED`; pass +`--confidence AMBIGUOUS` for a guess you want flagged, or `EXTRACTED` when the +source states it directly): ```bash -node scripts/enrichment/cli.js mapStep --workDir /tmp/pr-review-<pr> --function <name> --file <path> --step <canonicalName> -node scripts/enrichment/cli.js linkDiscussion --workDir /tmp/pr-review-<pr> --url <url> --source jira --title <title> -node scripts/enrichment/cli.js linkDoc --workDir /tmp/pr-review-<pr> --entity Step --name <name> --doc <path> +node scripts/enrichment/cli.js mapStep --workDir /tmp/pr-review-<pr> --function <name> --file <path> --step <canonicalName> [--confidence INFERRED|AMBIGUOUS|EXTRACTED] +node scripts/enrichment/cli.js linkDiscussion --workDir /tmp/pr-review-<pr> --url <url> --source jira --title <title> [--confidence ...] +node scripts/enrichment/cli.js linkDoc --workDir /tmp/pr-review-<pr> --entity Step --name <name> --doc <path> [--confidence ...] node scripts/enrichment/cli.js addGrammarRule --workDir /tmp/pr-review-<pr> --name <name> node scripts/enrichment/cli.js annotate --workDir /tmp/pr-review-<pr> --label Function --name <name> --key <key> --value <value> ``` diff --git a/.skills/tinker-review/references/schema.md b/.skills/tinker-review/references/schema.md index 03b833c3fb..22facca8d1 100644 --- a/.skills/tinker-review/references/schema.md +++ b/.skills/tinker-review/references/schema.md @@ -7,9 +7,25 @@ **File** `{ path, language, changed }` A source file in the PR. `changed: true` if modified in this PR. +*Stub Files* `{ path, language, changed: true, parsed: false, deleted }` are +markers for changed files the extractor didn't parse, so the PR's `modifies` +edge still lands. `deleted: true` means the PR removed the file (absent from the +PR-head worktree); `deleted: false` means it's present but an unparsed type +(non-code, or a non-primary language). Real Files have no `parsed` property, so +select materialized files with `.hasNot("parsed")` and markers with +`.has("parsed", false)`. + **Function** `{ name, signature, visibility, filePath, lines_start, lines_end, changed }` A function or method. The primary unit of analysis. +*External stub Functions* `{ name, external: true, resolved: false, changed: false }` +are markers created when a `calls`/`tests` edge targets a function by name that +wasn't extracted (a library/JDK call, or a function in a file this PR didn't +change). They keep the edge from vanishing and let blast-radius/centrality see +the call; they lack `filePath`/`signature`/`visibility`. Filter them out with +`.has("external", false)` — or, since real Functions have no `external` property, +`.hasNot("external")` — when you only want materialized code. + **Type** `{ name, kind, visibility, filePath }` A class, interface, struct, or enum. `kind` is one of: class, interface, struct, enum. @@ -43,6 +59,23 @@ A comment on a Discussion. ## Edges (14 labels) +### Edge confidence (every edge) + +Every edge carries a `confidence` property recording how the relationship was +established, so downstream analysis and the reviewer can separate observed fact +from deduction: + +| Value | Meaning | Examples | +|-------|---------|----------| +| `EXTRACTED` | Explicitly present in source or the git diff | `defines`, `modifies`, `has_comment`, an `addresses` link stated in the PR body/diff (`found_in: pr`/`diff`) | +| `INFERRED` | Reasonable deduction | `calls`/`tests` (resolved by name match), `proposed_in`, a cross-referenced `addresses` (`found_in: jira_body`/`devlist_body`), agent `implements_step`/`documents` mappings | +| `AMBIGUOUS` | Uncertain; flagged for human review | keyword-search `addresses` (`found_in: search`), low-confidence agent guesses | + +Enrichment write commands (`mapStep`, `linkDiscussion`, `linkDoc`) accept an +optional `--confidence` flag (default `INFERRED`). `auditConfidence` reports the +distribution and lists every `AMBIGUOUS` edge; the review's structural appendix +renders this as the **Signal Confidence** panel. + ### Code relationships | Edge | From | To | Meaning | diff --git a/.skills/tinker-review/scripts/enrichment/api.js b/.skills/tinker-review/scripts/enrichment/api.js index 5e6b370429..5508ed563b 100644 --- a/.skills/tinker-review/scripts/enrichment/api.js +++ b/.skills/tinker-review/scripts/enrichment/api.js @@ -20,6 +20,7 @@ import { readFile } from "node:fs/promises"; import { join } from "node:path"; import gremlin from "gremlin"; +import { CONFIDENCE, normalizeConfidence } from "../graph/confidence.js"; const { process: { statics: __ } } = gremlin; @@ -81,7 +82,8 @@ export async function getCanonicalSteps(repoPath) { // === Write operations === -export async function mapStep(g, functionName, filePath, canonicalStepName) { +export async function mapStep(g, functionName, filePath, canonicalStepName, confidence) { + const conf = normalizeConfidence(confidence, CONFIDENCE.INFERRED); const stepExists = await g.V().hasLabel("Step").has("name", canonicalStepName).hasNext(); if (!stepExists) { await g.addV("Step") @@ -94,13 +96,15 @@ export async function mapStep(g, functionName, filePath, canonicalStepName) { .has("name", functionName) .has("filePath", filePath) .addE("implements_step") + .property("confidence", conf) .to(__.V().hasLabel("Step").has("name", canonicalStepName)) .next(); - return { mapped: `${functionName} -> ${canonicalStepName}` }; + return { mapped: `${functionName} -> ${canonicalStepName}`, confidence: conf }; } -export async function linkDiscussion(g, url, source, title, body) { +export async function linkDiscussion(g, url, source, title, body, confidence) { + const conf = normalizeConfidence(confidence, CONFIDENCE.INFERRED); await g.addV("Discussion") .property("url", url) .property("source", source) @@ -112,11 +116,12 @@ export async function linkDiscussion(g, url, source, title, body) { if (prDiscussion) { await g.V().hasLabel("Discussion").has("source", "pr") .addE("addresses") + .property("confidence", conf) .to(__.V().hasLabel("Discussion").has("url", url)) .next(); } - return { linked: `${source}: ${title}` }; + return { linked: `${source}: ${title}`, confidence: conf }; } export async function annotate(g, label, name, key, value) { @@ -127,7 +132,8 @@ export async function annotate(g, label, name, key, value) { return { annotated: `${label}:${name}.${key} = ${value}` }; } -export async function linkDoc(g, entityLabel, entityName, docPath, section) { +export async function linkDoc(g, entityLabel, entityName, docPath, section, confidence) { + const conf = normalizeConfidence(confidence, CONFIDENCE.INFERRED); const docExists = await g.V().hasLabel("Doc").has("path", docPath).hasNext(); if (!docExists) { await g.addV("Doc") @@ -138,10 +144,11 @@ export async function linkDoc(g, entityLabel, entityName, docPath, section) { await g.V().hasLabel("Doc").has("path", docPath) .addE("documents") + .property("confidence", conf) .to(__.V().hasLabel(entityLabel).has("name", entityName)) .next(); - return { linked: `${docPath} documents ${entityLabel}:${entityName}` }; + return { linked: `${docPath} documents ${entityLabel}:${entityName}`, confidence: conf }; } export async function addGrammarRule(g, name, production) { diff --git a/.skills/tinker-review/scripts/enrichment/cli.js b/.skills/tinker-review/scripts/enrichment/cli.js index 4cf70ec498..c5f5099c22 100644 --- a/.skills/tinker-review/scripts/enrichment/cli.js +++ b/.skills/tinker-review/scripts/enrichment/cli.js @@ -25,12 +25,14 @@ import { mapStep, linkDiscussion, linkDoc, addGrammarRule, annotate, createPrDiscussion, } from "./api.js"; +import { confidenceAudit } from "../patterns/confidence-audit.js"; const COMMANDS = { listFunctions: { fn: listFunctions, needsG: true }, listTypes: { fn: listTypes, needsG: true }, getCallsFrom: { fn: getCallsFrom, needsG: true }, getCanonicalSteps: { fn: getCanonicalSteps, needsG: false }, + auditConfidence: { fn: confidenceAudit, needsG: true }, mapStep: { fn: mapStep, needsG: true }, linkDiscussion: { fn: linkDiscussion, needsG: true }, linkDoc: { fn: linkDoc, needsG: true }, @@ -73,9 +75,10 @@ async function main() { console.log(" listTypes [--kind class]"); console.log(" getCallsFrom --function <name> --file <path>"); console.log(" getCanonicalSteps"); - console.log(" mapStep --function <name> --file <path> --step <canonicalName>"); - console.log(" linkDiscussion --url <url> --source <jira|devlist|proposal> --title <title> [--body <body>]"); - console.log(" linkDoc --entity <label> --name <name> --doc <path> [--section <section>]"); + console.log(" auditConfidence [--maxAmbiguous 50]"); + console.log(" mapStep --function <name> --file <path> --step <canonicalName> [--confidence INFERRED|AMBIGUOUS|EXTRACTED]"); + console.log(" linkDiscussion --url <url> --source <jira|devlist|proposal> --title <title> [--body <body>] [--confidence ...]"); + console.log(" linkDoc --entity <label> --name <name> --doc <path> [--section <section>] [--confidence ...]"); console.log(" addGrammarRule --name <name> [--production <production>]"); console.log(" annotate --label <label> --name <name> --key <key> --value <value>"); console.log(""); @@ -124,14 +127,17 @@ async function main() { case "getCanonicalSteps": result = await fn(session.worktreePath || session.repoPath); break; + case "auditConfidence": + result = await fn(g, { maxAmbiguous: args.maxAmbiguous }); + break; case "mapStep": - result = await fn(g, args.function, args.file, args.step); + result = await fn(g, args.function, args.file, args.step, args.confidence); break; case "linkDiscussion": - result = await fn(g, args.url, args.source, args.title, args.body); + result = await fn(g, args.url, args.source, args.title, args.body, args.confidence); break; case "linkDoc": - result = await fn(g, args.entity, args.name, args.doc, args.section); + result = await fn(g, args.entity, args.name, args.doc, args.section, args.confidence); break; case "addGrammarRule": result = await fn(g, args.name, args.production); diff --git a/.skills/tinker-review/scripts/extraction/tree-sitter.js b/.skills/tinker-review/scripts/extraction/tree-sitter.js index 6b9a89833a..661a67ccc6 100644 --- a/.skills/tinker-review/scripts/extraction/tree-sitter.js +++ b/.skills/tinker-review/scripts/extraction/tree-sitter.js @@ -549,7 +549,15 @@ export async function extract(directory, language, options = {}) { }; for (const file of sourceFiles) { - const content = await readFile(file.fullPath, "utf-8"); + let content; + try { + content = await readFile(file.fullPath, "utf-8"); + } catch (err) { + // A file in the PR's changed set that isn't on disk was deleted by the + // PR — there's no source to extract, so skip it rather than crash. + if (err.code === "ENOENT") continue; + throw err; + } const tree = parser.parse(content); if (!tree) continue; diff --git a/.skills/tinker-review/scripts/graph/confidence.js b/.skills/tinker-review/scripts/graph/confidence.js new file mode 100644 index 0000000000..bb3076ceac --- /dev/null +++ b/.skills/tinker-review/scripts/graph/confidence.js @@ -0,0 +1,85 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +/** + * Edge confidence vocabulary. Every edge written into the knowledge graph + * carries a `confidence` property so downstream analysis (and the human + * reviewer) can tell what was observed directly from source versus what was + * deduced or guessed. + * + * EXTRACTED — explicitly present in the source: an AST definition, a git + * diff fact, a discussion link stated in the PR body/diff, a + * comment returned by the issue API for its parent thread. + * INFERRED — a reasonable deduction: a call/test edge resolved by name + * match, a cross-referenced discussion, an agent's step or doc + * mapping backed by evidence. + * AMBIGUOUS — uncertain; surfaced for human review. Keyword-search matches + * and low-confidence agent guesses land here. + */ +export const CONFIDENCE = Object.freeze({ + EXTRACTED: "EXTRACTED", + INFERRED: "INFERRED", + AMBIGUOUS: "AMBIGUOUS", +}); + +const VALID = new Set(Object.values(CONFIDENCE)); + +export function isValidConfidence(value) { + return VALID.has(value); +} + +/** + * Normalize a caller-supplied confidence value, falling back to a default when + * missing or invalid. Enrichment write commands use this so a bad `--confidence` + * flag degrades to a sane default rather than poisoning the graph. + * + * @param {string|undefined} value + * @param {string} [fallback=CONFIDENCE.INFERRED] + * @returns {string} + */ +export function normalizeConfidence(value, fallback = CONFIDENCE.INFERRED) { + if (typeof value === "string") { + const upper = value.toUpperCase(); + if (VALID.has(upper)) return upper; + } + return fallback; +} + +/** + * Map a discussion-link `found_in` provenance to a confidence level. + * A link stated in the PR body or diff is EXTRACTED; a cross-reference found in + * another discussion's body is INFERRED; a keyword-search hit is AMBIGUOUS. + * + * @param {string|undefined} foundIn + * @returns {string} + */ +export function confidenceForFoundIn(foundIn) { + switch (foundIn) { + case "pr": + case "diff": + return CONFIDENCE.EXTRACTED; + case "search": + return CONFIDENCE.AMBIGUOUS; + case "jira_body": + case "devlist_body": + return CONFIDENCE.INFERRED; + default: + return CONFIDENCE.INFERRED; + } +} diff --git a/.skills/tinker-review/scripts/graph/populate-discussions.js b/.skills/tinker-review/scripts/graph/populate-discussions.js index 996a3d5364..ea95c57f9d 100644 --- a/.skills/tinker-review/scripts/graph/populate-discussions.js +++ b/.skills/tinker-review/scripts/graph/populate-discussions.js @@ -18,6 +18,7 @@ */ import gremlin from "gremlin"; +import { CONFIDENCE, confidenceForFoundIn } from "./confidence.js"; const { process: { statics: __ } } = gremlin; @@ -49,15 +50,21 @@ export async function populateDiscussions(g, discussions, context) { const prUrl = `https://github.com/apache/tinkerpop/pull/${context.pr}`; - // Create the PR itself as a Discussion vertex - await g.addV("Discussion") - .property("url", prUrl) - .property("source", "pr") - .property("title", context.prTitle || `PR #${context.pr}`) - .property("body", "") - .next(); - counts.vertices++; - counts.breakdown.discussions++; + // Create the PR itself as a Discussion vertex. Idempotent: review.js may have + // already created it via createPrDiscussion(). A duplicate PR vertex would make + // `.V().has("url", prUrl)` match two start vertices and double every PR-sourced + // edge (modifies, addresses, has_comment). + const prExists = await g.V().hasLabel("Discussion").has("url", prUrl).hasNext(); + if (!prExists) { + await g.addV("Discussion") + .property("url", prUrl) + .property("source", "pr") + .property("title", context.prTitle || `PR #${context.pr}`) + .property("body", "") + .next(); + counts.vertices++; + counts.breakdown.discussions++; + } // Create Discussion vertices for JIRAs for (const jira of discussions.jiras) { @@ -158,6 +165,7 @@ export async function populateDiscussions(g, discussions, context) { batch.push( g.V().hasLabel("Discussion").has("url", prUrl) .addE("has_comment") + .property("confidence", CONFIDENCE.EXTRACTED) .to(__.V().hasLabel("Comment").has("author", comment.author).has("timestamp", comment.timestamp || "")) ); counts.edges++; @@ -169,6 +177,7 @@ export async function populateDiscussions(g, discussions, context) { batch.push( g.V().hasLabel("Discussion").has("url", prUrl) .addE("has_comment") + .property("confidence", CONFIDENCE.EXTRACTED) .to(__.V().hasLabel("Comment").has("author", comment.author).has("timestamp", comment.timestamp || "")) ); counts.edges++; @@ -182,6 +191,7 @@ export async function populateDiscussions(g, discussions, context) { batch.push( g.V().hasLabel("Discussion").has("url", jira.url) .addE("has_comment") + .property("confidence", CONFIDENCE.EXTRACTED) .to(__.V().hasLabel("Comment").has("author", comment.author).has("timestamp", comment.timestamp || "")) ); counts.edges++; @@ -196,6 +206,7 @@ export async function populateDiscussions(g, discussions, context) { batch.push( g.V().hasLabel("Discussion").has("url", sec.url) .addE("has_comment") + .property("confidence", CONFIDENCE.EXTRACTED) .to(__.V().hasLabel("Comment").has("author", comment.author).has("timestamp", comment.timestamp || "")) ); counts.edges++; @@ -210,6 +221,7 @@ export async function populateDiscussions(g, discussions, context) { g.V().hasLabel("Discussion").has("url", prUrl) .addE("addresses") .property("found_in", jira.found_in || "pr") + .property("confidence", confidenceForFoundIn(jira.found_in || "pr")) .to(__.V().hasLabel("Discussion").has("url", jira.url)) ); counts.edges++; @@ -223,6 +235,7 @@ export async function populateDiscussions(g, discussions, context) { g.V().hasLabel("Discussion").has("url", prUrl) .addE("addresses") .property("found_in", thread.found_in || "pr") + .property("confidence", confidenceForFoundIn(thread.found_in || "pr")) .to(__.V().hasLabel("Discussion").has("url", thread.url)) ); counts.edges++; @@ -240,6 +253,7 @@ export async function populateDiscussions(g, discussions, context) { .addE("addresses") .property("found_in", sec.found_in || "") .property("found_via", sec.found_via || "") + .property("confidence", confidenceForFoundIn(sec.found_in)) .to(__.V().hasLabel("Discussion").has("url", sec.url)) ); counts.edges++; @@ -252,6 +266,7 @@ export async function populateDiscussions(g, discussions, context) { batch.push( g.V().hasLabel("Discussion").has("source", "proposal").has("title", proposal.title) .addE("proposed_in") + .property("confidence", CONFIDENCE.INFERRED) .to(__.V().hasLabel("Discussion").has("url", prUrl)) ); counts.edges++; @@ -264,6 +279,7 @@ export async function populateDiscussions(g, discussions, context) { batch.push( g.V().hasLabel("Discussion").has("url", prUrl) .addE("modifies") + .property("confidence", CONFIDENCE.EXTRACTED) .to(__.V().hasLabel("File").has("path", filePath)) ); counts.edges++; diff --git a/.skills/tinker-review/scripts/graph/populate.js b/.skills/tinker-review/scripts/graph/populate.js index 748dda561f..bc06379fbe 100644 --- a/.skills/tinker-review/scripts/graph/populate.js +++ b/.skills/tinker-review/scripts/graph/populate.js @@ -17,7 +17,10 @@ * under the License. */ +import { existsSync } from "node:fs"; +import { join } from "node:path"; import gremlin from "gremlin"; +import { CONFIDENCE } from "./confidence.js"; const { process: { statics: __ } } = gremlin; @@ -34,13 +37,19 @@ async function submitBatch(batch) { * * @param {object} g - gremlin-js GraphTraversalSource (already connected) * @param {ExtractionResult} extraction - Output from tree-sitter module + * @param {object} [options] + * @param {string[]} [options.changedFiles] - Full changed-file list for the PR + * (used to mark files the PR touched but the extractor didn't parse) + * @param {string} [options.worktreePath] - PR-head worktree, to tell a deleted + * file (absent on disk) from an unparsed one (present but not a parsed language) * @returns {Promise<PopulationSummary>} */ -export async function populate(g, extraction) { +export async function populate(g, extraction, options = {}) { + const { changedFiles = [], worktreePath = "" } = options; const counts = { vertices: 0, edges: 0, - breakdown: { files: 0, functions: 0, types: 0, tests: 0, calls: 0, defines: 0, testsEdges: 0 }, + breakdown: { files: 0, functions: 0, types: 0, tests: 0, calls: 0, defines: 0, testsEdges: 0, externalFunctions: 0, stubFiles: 0 }, }; for (const file of extraction.files) { @@ -53,6 +62,30 @@ export async function populate(g, extraction) { counts.breakdown.files++; } + // Mark changed files the extractor didn't parse. The PR's `modifies` edges + // target every changed file by path, but only parsed files (right language, + // present on disk) get a File vertex above — so a deleted or non-code file + // would have no vertex to land on and its `modifies` edge would vanish. Create + // a stub File as a marker (keyed by unique path, so it's race-free). A file + // absent from the PR-head worktree was deleted by the PR; one still on disk was + // simply not parsed (unsupported language). These markers are especially + // meaningful on removal PRs — they record exactly what the PR took out. + const extractedPaths = new Set(extraction.files.map((f) => f.path)); + for (const filePath of changedFiles) { + if (extractedPaths.has(filePath)) continue; + const onDisk = worktreePath ? existsSync(join(worktreePath, filePath)) : true; + const ext = filePath.includes(".") ? filePath.split(".").pop() : ""; + await g.addV("File") + .property("path", filePath) + .property("language", ext) + .property("changed", true) + .property("parsed", false) + .property("deleted", !onDisk) + .next(); + counts.vertices++; + counts.breakdown.stubFiles++; + } + for (const fn of extraction.functions) { await g.addV("Function") .property("name", fn.name) @@ -91,10 +124,51 @@ export async function populate(g, extraction) { let batch = []; + // Resolve-or-mark callee vertices. Call/test edges target a function by name; + // when the callee isn't among the extracted functions (a library/JDK call, or + // a function in a file this PR didn't change) there is no vertex to land on + // and the edge would silently vanish. Materialize a lightweight "external" + // stub as a marker so the edge survives and downstream analysis (blast radius, + // centrality) can see the call. Stubs are keyed by unique name and created + // up front, so this is idempotent and race-free even under batched inserts. + // These MUST be flushed before the calls/tests edges below reference them. + const extractedFunctionNames = new Set(extraction.functions.map((f) => f.name)); + const unresolvedCallees = new Set(); + for (const call of extraction.calls) { + if (!extractedFunctionNames.has(call.calleeName)) unresolvedCallees.add(call.calleeName); + } + for (const test of tests) { + for (const calledFn of (test.calledFunctions || [])) { + if (!extractedFunctionNames.has(calledFn)) unresolvedCallees.add(calledFn); + } + } + + for (const name of unresolvedCallees) { + batch.push( + g.addV("Function") + .property("name", name) + .property("external", true) + .property("resolved", false) + .property("changed", false) + ); + counts.vertices++; + counts.breakdown.externalFunctions++; + + if (batch.length >= BATCH_SIZE) { + await submitBatch(batch); + batch = []; + } + } + if (batch.length > 0) { + await submitBatch(batch); + batch = []; + } + for (const fn of extraction.functions) { batch.push( g.V().hasLabel("File").has("path", fn.filePath) .addE("defines") + .property("confidence", CONFIDENCE.EXTRACTED) .to(__.V().hasLabel("Function").has("name", fn.name).has("filePath", fn.filePath)) ); counts.edges++; @@ -110,6 +184,7 @@ export async function populate(g, extraction) { batch.push( g.V().hasLabel("File").has("path", type.filePath) .addE("defines") + .property("confidence", CONFIDENCE.EXTRACTED) .to(__.V().hasLabel("Type").has("name", type.name).has("filePath", type.filePath)) ); counts.edges++; @@ -122,11 +197,15 @@ export async function populate(g, extraction) { } for (const call of extraction.calls) { + // INFERRED: the call site is real, but the callee is resolved by name alone + // (it matches any Function with that name, across files/overloads), so the + // edge target is a deduction rather than a directly observed fact. batch.push( g.V().hasLabel("Function") .has("name", call.callerName) .has("filePath", call.callerFile) .addE("calls") + .property("confidence", CONFIDENCE.INFERRED) .to(__.V().hasLabel("Function").has("name", call.calleeName)) ); counts.edges++; @@ -140,9 +219,11 @@ export async function populate(g, extraction) { for (const test of tests) { for (const calledFn of test.calledFunctions) { + // INFERRED: a test is linked to a function by name match on the callee. batch.push( g.V().hasLabel("Test").has("name", test.name).has("filePath", test.filePath) .addE("tests") + .property("confidence", CONFIDENCE.INFERRED) .to(__.V().hasLabel("Function").has("name", calledFn)) ); counts.edges++; @@ -166,5 +247,14 @@ export async function populate(g, extraction) { await submitBatch(batch); } + // Report the true graph size. The per-type breakdown above counts attempted + // inserts; query the graph itself for the authoritative vertex/edge totals so + // the summary can't drift from reality (e.g. an edge whose endpoints matched + // multiple vertices, or a vertex insert that failed). + const realV = await g.V().count().next(); + const realE = await g.E().count().next(); + counts.vertices = Number(realV.value); + counts.edges = Number(realE.value); + return counts; } diff --git a/.skills/tinker-review/scripts/patterns/confidence-audit.js b/.skills/tinker-review/scripts/patterns/confidence-audit.js new file mode 100644 index 0000000000..ddb5c6e30d --- /dev/null +++ b/.skills/tinker-review/scripts/patterns/confidence-audit.js @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import gremlin from "gremlin"; +import { CONFIDENCE } from "../graph/confidence.js"; + +const { process: { statics: __ } } = gremlin; + +// Pick a human-readable label for a vertex from whichever identifying property +// it carries. Different labels key on different properties (Function/Type/Step +// use name, File uses path, Discussion uses url/title). +function describeVertex(elementMap) { + if (!elementMap) return "?"; + const get = (k) => { + const v = elementMap.get ? elementMap.get(k) : elementMap[k]; + return v == null ? undefined : String(v); + }; + const labelKey = gremlin.process.t.label; + const label = (elementMap.get ? elementMap.get(labelKey) : elementMap["label"]) || ""; + const name = get("name") || get("title") || get("path") || get("url") || "?"; + return label ? `${label}(${name})` : name; +} + +/** + * Audit the knowledge graph by edge confidence. Returns the distribution across + * EXTRACTED / INFERRED / AMBIGUOUS (plus UNTAGGED for any edge missing the + * property) and the full list of AMBIGUOUS edges — the ones flagged for human + * review. Since it reads the live graph it reflects enrichment edges too when + * run after Phase 2. + * + * @param {object} g - gremlin-js GraphTraversalSource (already connected) + * @param {object} [params] + * @param {number} [params.maxAmbiguous] - Cap the AMBIGUOUS edge list (default 50) + * @returns {Promise<{distribution: object, total: number, ambiguous: object[]}>} + */ +export async function confidenceAudit(g, params = {}) { + const maxAmbiguous = params.maxAmbiguous || 50; + + // Distribution over every edge; edges predating this feature (or any we missed) + // fall into UNTAGGED so the audit stays honest about coverage. + const grouped = await g.E() + .groupCount() + .by(__.coalesce(__.values("confidence"), __.constant("UNTAGGED"))) + .next(); + + const distribution = { EXTRACTED: 0, INFERRED: 0, AMBIGUOUS: 0, UNTAGGED: 0 }; + let total = 0; + const gm = grouped.value; + if (gm) { + const entries = gm instanceof Map ? gm.entries() : Object.entries(gm); + for (const [key, count] of entries) { + const n = Number(count); + distribution[key] = (distribution[key] || 0) + n; + total += n; + } + } + + const ambiguousRows = await g.E() + .has("confidence", CONFIDENCE.AMBIGUOUS) + .project("relation", "found_in", "found_via", "from", "to") + .by(__.label()) + .by(__.coalesce(__.values("found_in"), __.constant(""))) + .by(__.coalesce(__.values("found_via"), __.constant(""))) + .by(__.outV().elementMap()) + .by(__.inV().elementMap()) + .limit(maxAmbiguous) + .toList(); + + const ambiguous = ambiguousRows.map((row) => ({ + relation: row.get("relation"), + from: describeVertex(row.get("from")), + to: describeVertex(row.get("to")), + foundIn: row.get("found_in") || undefined, + foundVia: row.get("found_via") || undefined, + })); + + return { distribution, total, ambiguous }; +} diff --git a/.skills/tinker-review/scripts/renderer/render.js b/.skills/tinker-review/scripts/renderer/render.js index ae18e32829..dbd71adbdb 100644 --- a/.skills/tinker-review/scripts/renderer/render.js +++ b/.skills/tinker-review/scripts/renderer/render.js @@ -39,7 +39,7 @@ function esc(str) { * Input contract — data fields (from review.js): * meta: { pr, title, domains: [], language, changedFileCount, timestamp } * graphStats: { vertices, edges, breakdown: { files, functions, types, tests, calls } } - * checks: { completeness, coverageGaps, centrality, blastRadius, clusters } + * checks: { completeness, coverageGaps, centrality, blastRadius, clusters, confidence } * discussions: { jiras, devList, secondary, prComments, devListSearchKeywords, ... } * changedFiles: [] * @@ -345,11 +345,53 @@ function renderOpenQuestions(questions) { return `<section id="open-questions">\n <h2>Open Questions</h2>\n ${cards}\n</section>`; } +function renderConfidence(confidence) { + if (!confidence || !confidence.distribution) return ""; + const d = confidence.distribution; + const ambiguous = confidence.ambiguous || []; + + const chip = (label, value, cls) => + `<div class="stat-box"><div class="value">${value || 0}</div><div class="label">${label}</div></div>`; + + let ambiguousHtml; + if (ambiguous.length === 0) { + ambiguousHtml = `<p class="section-intro">No AMBIGUOUS edges — nothing was linked purely by keyword search or low-confidence guess.</p>`; + } else { + const rows = ambiguous.map(a => { + const via = a.foundVia ? ` <span class="discovery-meta">via ${esc(a.foundVia)}</span>` : ""; + const found = a.foundIn ? esc(a.foundIn) : "—"; + return `<tr><td><code>${esc(a.relation)}</code></td><td class="fn-name">${esc(a.from)}</td><td class="fn-name">${esc(a.to)}${via}</td><td>${found}</td></tr>`; + }).join("\n "); + ambiguousHtml = `<table class="gap-table"> + <thead><tr><th>Relation</th><th>From</th><th>To</th><th>Found in</th></tr></thead> + <tbody>\n ${rows}\n </tbody> + </table>`; + } + + const untagged = d.UNTAGGED + ? `<div class="stat-box"><div class="value">${d.UNTAGGED}</div><div class="label">Untagged</div></div>` + : ""; + + return ` + <h3>Signal Confidence</h3> + <p class="section-intro">Every graph edge is tagged by how it was established. <strong>EXTRACTED</strong> edges are observed directly in source or the git diff; <strong>INFERRED</strong> edges are name-resolved or evidence-backed deductions; <strong>AMBIGUOUS</strong> edges are keyword-search or low-confidence guesses and are listed below for human review.</p> + <div class="stats-grid"> + ${chip("Extracted", d.EXTRACTED)} + ${chip("Inferred", d.INFERRED)} + ${chip("Ambiguous", d.AMBIGUOUS)} + ${untagged} + </div> + <h4 style="margin-top: 1rem;">Ambiguous edges (${ambiguous.length}) — verify before relying on</h4> + ${ambiguousHtml} +`; +} + function renderAppendixStructural(checks, graphStats) { const hotspots = checks?.centrality?.hotspots || []; const blast = checks?.blastRadius?.functions || []; const stats = graphStats || {}; const bd = stats.breakdown || {}; + const confidenceHtml = renderConfidence(checks?.confidence); const hotspotRows = hotspots.slice(0, 10).map(h => { const badge = (h.inherentlyCentral && h.changed) ? `<span class="badge badge-modified">modified</span>` : ""; @@ -377,7 +419,7 @@ function renderAppendixStructural(checks, graphStats) { <thead><tr><th>Function</th><th>File</th><th>Reachable</th><th></th></tr></thead> <tbody>\n ${blastRows}\n </tbody> </table> - +${confidenceHtml} <h3>Graph Statistics</h3> <div class="stats-grid"> <div class="stat-box"><div class="value">${stats.vertices || 0}</div><div class="label">Vertices</div></div> diff --git a/.skills/tinker-review/scripts/review.js b/.skills/tinker-review/scripts/review.js index ebe50ffe99..bda97cd7b8 100644 --- a/.skills/tinker-review/scripts/review.js +++ b/.skills/tinker-review/scripts/review.js @@ -34,6 +34,7 @@ import { highCentrality } from "./patterns/centrality.js"; import { blastRadius } from "./patterns/blast-radius.js"; import { clusterAnalysis } from "./patterns/cluster-analysis.js"; import { architecture } from "./patterns/architecture.js"; +import { confidenceAudit } from "./patterns/confidence-audit.js"; import { createPrDiscussion } from "./enrichment/api.js"; import { discoverDiscussions } from "./discovery/discussions.js"; @@ -234,7 +235,8 @@ export async function setup(params) { // ============================================================ // PHASE 1 — extract, populate, discover, run checks -// Writes evidence JSON to workDir. Server stays alive. +// Writes evidence JSON to workDir. The Gremlin Server container stays alive for +// enrichment; the CLI process itself exits once Phase 1 completes (see main()). // ============================================================ export async function phase1(session) { @@ -245,7 +247,7 @@ export async function phase1(session) { log(`Phase 1 complete: ${extraction.files.length} files, ${extraction.functions.length} functions, ${extraction.types.length} types`); log(`Populating graph...`); - const graphStats = await populate(g, extraction); + const graphStats = await populate(g, extraction, { changedFiles, worktreePath }); log(`Graph populated: ${graphStats.vertices} vertices, ${graphStats.edges} edges`); let prTitle = `PR #${pr}`; @@ -303,11 +305,13 @@ export async function phase1(session) { const centralityResult = await highCentrality(g, { changedOnly: true, topN: 10, minDegree: 3 }); const blastResult = await blastRadius(g, { depth: 3, changedOnly: true }); const clusterResult = await clusterAnalysis(a, { changedOnly: true }); + const confidenceResult = await confidenceAudit(g); log(` completeness: ${completenessResults.filter(r => r.missing.length > 0).length} gaps found`); log(` coverage_gaps: ${coverageResult.uncovered.length} functions without tests`); log(` centrality: ${centralityResult.aboveThreshold} hotspots`); log(` blast_radius: max ${blastResult.maxReachable} reachable`); log(` clusters: ${clusterResult.clusterCount} (${clusterResult.coherent ? "coherent" : "fragmented"})`); + log(` confidence: ${confidenceResult.distribution.EXTRACTED} extracted / ${confidenceResult.distribution.INFERRED} inferred / ${confidenceResult.distribution.AMBIGUOUS} ambiguous`); log(`Generating architecture map...`); const architectureResult = await architecture(g, { clusterResult, changedOnly: true }); @@ -330,6 +334,7 @@ export async function phase1(session) { centrality: centralityResult, blastRadius: blastResult, clusters: clusterResult, + confidence: confidenceResult, }, discussions, changedFiles, @@ -399,6 +404,16 @@ if (process.argv[1] && basename(process.argv[1]) === "review.js") { log(`Work directory: ${session.workDir}`); log(`Worktree: ${session.worktreePath}`); log(`To teardown: call teardown(session) or stop container ${session.handle.containerId}`); + + // Phase 1 is done. Close OUR Gremlin connections so this process exits + // cleanly and completion is detectable (exit code 0 + the sentinel line + // below). The Gremlin Server container stays up independently — enrichment + // (scripts/enrichment/cli.js) opens its own connection per command from + // session.json, so nothing depends on this process lingering. + await session.connection.close().catch(() => {}); + await session.aConnection.close().catch(() => {}); + log(`PHASE1_COMPLETE pr=${pr} evidence=${jsonPath} port=${session.handle.port} container=${session.handle.containerId}`); + process.exit(0); } catch (err) { await teardown(session).catch(() => {}); throw err;
