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 e2e3a5f6cf278353930e6ad6092fb0c31386c5b7 Author: Stephen Mallette <[email protected]> AuthorDate: Sun Jul 5 09:32:31 2026 -0400 tinker-review: type-aware blast radius through the type hierarchy Blast radius followed only call edges, so a change to an interface method read as low-impact even though its real reach is every override. The extractor now captures the type hierarchy — extends/implements (Type->Type), declares (Type->Function), and derived overrides (Function->Function) — and the blast-radius walk follows overrides alongside calls, plus a type-seeded pass that reports the functions reachable through a changed interface's implementers as a separate bucket. Because review extracts only changed files, a narrow PR would otherwise leave overriders (or ancestors) unparsed and undercount to zero. The extractor now pulls in the type-hierarchy neighborhood of changed types as context — a bounded downward/upward walk over a cheap regex index of the worktree, capped and surfaced as a lower bound when truncated. Also closes two enrichment dead ends: adds linkRule (has_rule) and mapCoverage (covers) so the grammar and new-step playbooks can create the edges their completeness checks look for, and trues schema.md against what the code actually produces (drops the never-built GLV/provides). Assisted-by: Claude Code:claude-opus-4-8 --- .skills/tinker-review/playbooks/grammar.md | 5 +- .skills/tinker-review/playbooks/new-step.md | 2 + .skills/tinker-review/references/enrichment-cli.md | 12 + .skills/tinker-review/references/schema.md | 35 ++- .skills/tinker-review/scripts/enrichment/api.js | 69 +++++ .skills/tinker-review/scripts/enrichment/cli.js | 12 +- .../scripts/extraction/tree-sitter.js | 333 ++++++++++++++++++--- .skills/tinker-review/scripts/graph/populate.js | 119 +++++++- .../tinker-review/scripts/patterns/blast-radius.js | 79 ++++- .skills/tinker-review/scripts/renderer/render.js | 17 +- .skills/tinker-review/scripts/review.js | 7 +- .skills/tinker-review/test/extraction.test.js | 151 ++++++++++ 12 files changed, 765 insertions(+), 76 deletions(-) diff --git a/.skills/tinker-review/playbooks/grammar.md b/.skills/tinker-review/playbooks/grammar.md index 5aad50efe1..f402817cc0 100644 --- a/.skills/tinker-review/playbooks/grammar.md +++ b/.skills/tinker-review/playbooks/grammar.md @@ -6,8 +6,9 @@ are inherently high-risk — they affect all parsers, all GLVs, and all downstream tooling. Backwards compatibility is critical. ## Enrich -- `addGrammarRule` — record each grammar rule the PR adds, so completeness can - check it's wired to a step. +- `addGrammarRule` — record each grammar rule the PR adds. +- `linkRule` — wire each rule to the step it defines (`has_rule`), so + completeness can check every rule reaches a step. - `linkDiscussion --source proposal` — record the proposal or dev-list thread (grammar changes need prior community consensus). diff --git a/.skills/tinker-review/playbooks/new-step.md b/.skills/tinker-review/playbooks/new-step.md index 75a4393ce6..6db570c2d7 100644 --- a/.skills/tinker-review/playbooks/new-step.md +++ b/.skills/tinker-review/playbooks/new-step.md @@ -11,6 +11,8 @@ semantics. - `mapStep` — link the core implementation to its canonical name. Map the `GraphTraversal`/`GraphTraversalSource` method users call, not the internal methods on the Step class (e.g., `TreeStep`). +- `mapCoverage` — record each test that exercises the step (`covers`), so + completeness/coverageGaps can confirm the new step is tested. - `linkDoc` — record the documentation that references the step. - `linkDiscussion` — record the proposal or JIRA that defines the step's semantics. diff --git a/.skills/tinker-review/references/enrichment-cli.md b/.skills/tinker-review/references/enrichment-cli.md index 53ae52de5c..044108451e 100644 --- a/.skills/tinker-review/references/enrichment-cli.md +++ b/.skills/tinker-review/references/enrichment-cli.md @@ -146,6 +146,18 @@ Adds a `GrammarRule` vertex for a rule the PR introduces. Reach for it in the grammar playbook when a `*.g4` change adds a production the graph should track so later steps can link to it. +### linkRule +Links a step to the grammar production that defines it, as a `has_rule` edge +(Step → GrammarRule). Closes the loop `addGrammarRule` opens — both vertices must +exist first (`mapStep` for the Step, `addGrammarRule` for the rule). The grammar +playbook's `checks.completeness` on `has_rule` reports which steps still lack it. + +### mapCoverage +Records that a test covers a step's behavior, as a `covers` edge (Test → Step). +Key the test by `--test` name **and** `--file` (names repeat across suites); the +Step must already exist (`mapStep`). The `new-step` playbook treats a step with +no `covers` as a coverage gap — this is how you record the coverage you find. + ### annotate Sets an arbitrary `--key`/`--value` property on a vertex identified by `--label` + `--name`. The **general-purpose escape hatch** for a fact the schema has no diff --git a/.skills/tinker-review/references/schema.md b/.skills/tinker-review/references/schema.md index 38564f8aa9..0f787a34de 100644 --- a/.skills/tinker-review/references/schema.md +++ b/.skills/tinker-review/references/schema.md @@ -32,8 +32,17 @@ name — noise), `project` (a repo source declares a type with this name; `origin: library` calls from out-degree so ubiquitous accessor calls don't inflate hotspots. -**Type** `{ name, kind, visibility, filePath }` -A class, interface, struct, or enum. `kind` is one of: class, interface, struct, enum. +**Type** `{ name, kind, visibility, filePath, changed }` +A class, interface, struct, or enum. `kind` is one of: class, interface, struct, +enum. `changed: true` if the PR modified the file it's declared in. + +*External stub Types* `{ name, external: true, resolved: false }` are markers +created when an `extends`/`implements` edge names a supertype that wasn't +extracted — typically a JDK/library class outside the worktree. In-repo +supertypes are usually materialized by the extractor's hierarchy-neighborhood +expansion (which pulls a changed type's ancestors and descendants in as context), +so stubs mostly stand for third-party types. They lack `kind`/`filePath`; filter +them with `.has("external", false)` or `.hasNot("external")`. ### TinkerPop domain @@ -43,12 +52,9 @@ A Gremlin traversal step as a concept (e.g., "addV", "has", "out"). Created duri **GrammarRule** `{ name, production }` An ANTLR production in Gremlin.g4. -**GLV** `{ language }` -A Gremlin Language Variant (e.g., "dart", "go", "python"). - ### Verification -**Test** `{ name, type }` +**Test** `{ name, type, filePath }` A test function. `type` is one of: unit, integration, suite. **Doc** `{ path, section }` @@ -63,7 +69,10 @@ The PR itself is a Discussion with `source: "pr"`. **Comment** `{ author, body, timestamp }` A comment on a Discussion. -## Edges (15 labels) +## Edges (16 implemented + 1 planned) + +One edge is marked ⚠️ *planned* below (`depends_on`) — documented but intentionally +not populated. ### Edge confidence (every edge) @@ -88,8 +97,11 @@ renders this as the **Signal Confidence** panel. |------|------|----|---------| | `calls` | Function | Function | Function invokes another function | | `defines` | File | Function or Type | File contains this definition | -| `implements` | Function | Type | Function implements an interface | -| `depends_on` | File | File | File imports/requires another file | +| `declares` | Type | Function | Type's body declares this method (the membership edge; both endpoints pinned to the same file). `EXTRACTED` | +| `extends` | Type | Type | Subclass extends a superclass, or interface extends an interface. Supertype resolved by simple name; unresolved parents get an external Type stub. `INFERRED` | +| `implements` | Type | Type | Class implements an interface. Same resolution/stub behavior as `extends`. (Java splits `extends`/`implements` precisely; other languages label all bases `extends`.) `INFERRED` | +| `overrides` | Function | Function | A method overrides a same-named method declared by an ancestor type (transitive over `extends`/`implements`). Derived after population by `deriveOverrides`. `INFERRED` | +| `depends_on` | File | File | ⚠️ *planned, not populated.* File imports/requires another file. Intentionally omitted — call/defines edges already carry file connectivity (see `populate.js`). | | `references` | File | File (deleted) | A surviving file still mentions a symbol from a file the PR deleted. Added during a removal review via `addReference`; carries `symbol` and `location` properties. | ### Domain relationships @@ -97,15 +109,14 @@ renders this as the **Signal Confidence** panel. | Edge | From | To | Meaning | |------|------|----|---------| | `implements_step` | Function | Step | This function is a GLV's implementation of a Gremlin step | -| `has_rule` | Step | GrammarRule | This step is defined by this grammar production | -| `provides` | GLV | Step | This GLV implements this step | +| `has_rule` | Step | GrammarRule | This step is defined by this grammar production. Written by `linkRule` (after `addGrammarRule` creates the rule vertex). `INFERRED` | ### Verification | Edge | From | To | Meaning | |------|------|----|---------| | `tests` | Test | Function | This test exercises this function | -| `covers` | Test | Step | This test covers this step's behavior | +| `covers` | Test | Step | This test covers this step's behavior. Written by `mapCoverage`. The `new-step` playbook treats a missing `covers` as a coverage gap. `INFERRED` | | `documents` | Doc | Step, Function, or Type | This doc describes this entity | ### Discussion diff --git a/.skills/tinker-review/scripts/enrichment/api.js b/.skills/tinker-review/scripts/enrichment/api.js index 460b15619d..c9b6c446d0 100644 --- a/.skills/tinker-review/scripts/enrichment/api.js +++ b/.skills/tinker-review/scripts/enrichment/api.js @@ -388,6 +388,75 @@ export async function addGrammarRule(g, name, production) { return { added: `GrammarRule: ${name}` }; } +/** + * Link a Gremlin step to the grammar production that defines it, as a `has_rule` + * edge (Step -> GrammarRule). Closes the loop `addGrammarRule` opens: the rule + * vertex exists, this connects the step concept to it. The grammar playbook's + * `checks.completeness` on `has_rule` reports which steps still lack this link. + * Both vertices must already exist (`mapStep` creates the Step, `addGrammarRule` + * the rule). + * + * @param {object} g - gremlin-js GraphTraversalSource (already connected) + * @param {string} stepName - Canonical step name (an existing Step vertex) + * @param {string} ruleName - Grammar rule name (an existing GrammarRule vertex) + * @param {string} [confidence] - default INFERRED + * @returns {Promise<object>} + */ +export async function linkRule(g, stepName, ruleName, confidence) { + const conf = normalizeConfidence(confidence, CONFIDENCE.INFERRED); + const stepExists = await g.V().hasLabel("Step").has("name", stepName).hasNext(); + if (!stepExists) { + return { error: `no Step vertex named "${stepName}" (map it with mapStep first)` }; + } + const ruleExists = await g.V().hasLabel("GrammarRule").has("name", ruleName).hasNext(); + if (!ruleExists) { + return { error: `no GrammarRule vertex named "${ruleName}" (add it with addGrammarRule first)` }; + } + + await g.V().hasLabel("Step").has("name", stepName) + .addE("has_rule") + .property("confidence", conf) + .to(__.V().hasLabel("GrammarRule").has("name", ruleName)) + .next(); + + return { linked: `${stepName} has_rule ${ruleName}`, confidence: conf }; +} + +/** + * Record that a test covers a Gremlin step's behavior, as a `covers` edge + * (Test -> Step). The `new-step` playbook treats a step with no `covers` as a + * coverage gap; this is how you record the coverage you find during enrichment. + * The test is keyed by name AND file because test names repeat across suites; + * the Step vertex must already exist (created via `mapStep`). + * + * @param {object} g - gremlin-js GraphTraversalSource (already connected) + * @param {string} testName - The covering test's name + * @param {string} filePath - The test's file (disambiguates same-named tests) + * @param {string} stepName - Canonical step name (an existing Step vertex) + * @param {string} [confidence] - default INFERRED + * @returns {Promise<object>} + */ +export async function mapCoverage(g, testName, filePath, stepName, confidence) { + const conf = normalizeConfidence(confidence, CONFIDENCE.INFERRED); + const stepExists = await g.V().hasLabel("Step").has("name", stepName).hasNext(); + if (!stepExists) { + return { error: `no Step vertex named "${stepName}" (map it with mapStep first)` }; + } + const testExists = await g.V().hasLabel("Test") + .has("name", testName).has("filePath", filePath).hasNext(); + if (!testExists) { + return { error: `no Test vertex "${testName}" in ${filePath}` }; + } + + await g.V().hasLabel("Test").has("name", testName).has("filePath", filePath) + .addE("covers") + .property("confidence", conf) + .to(__.V().hasLabel("Step").has("name", stepName)) + .next(); + + return { covered: `${testName} covers ${stepName}`, confidence: conf }; +} + /** * INTERNAL (Phase 1). Create the root PR Discussion vertex that every other * discussion links back to via `addresses`. review.js calls this once while diff --git a/.skills/tinker-review/scripts/enrichment/cli.js b/.skills/tinker-review/scripts/enrichment/cli.js index 464f0ab214..989454d189 100644 --- a/.skills/tinker-review/scripts/enrichment/cli.js +++ b/.skills/tinker-review/scripts/enrichment/cli.js @@ -25,7 +25,7 @@ import { listFunctions, listTypes, getCallsFrom, getCanonicalSteps, listDeleted, listExternalRefs, addReference, mapStep, setEdgeConfidence, linkDiscussion, linkDoc, addGrammarRule, annotate, - createPrDiscussion, + linkRule, mapCoverage, createPrDiscussion, } from "./api.js"; import { confidenceAudit, listInferred } from "../patterns/confidence-audit.js"; import { classifyExternals } from "../patterns/classify-externals.js"; @@ -54,6 +54,8 @@ const COMMANDS = { linkDiscussion: { fn: linkDiscussion, needsG: true, facing: "agent" }, linkDoc: { fn: linkDoc, needsG: true, facing: "agent" }, addGrammarRule: { fn: addGrammarRule, needsG: true, facing: "agent" }, + linkRule: { fn: linkRule, needsG: true, facing: "agent" }, + mapCoverage: { fn: mapCoverage, needsG: true, facing: "agent" }, annotate: { fn: annotate, needsG: true, facing: "agent" }, // Internal — run by review.js during Phase 1; exposed for manual re-runs only classifyExternals: { fn: classifyExternals, needsG: true, facing: "internal" }, @@ -104,6 +106,8 @@ async function main() { 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(" linkRule --step <canonicalName> --rule <ruleName> [--confidence ...]"); + console.log(" mapCoverage --test <name> --file <path> --step <canonicalName> [--confidence ...]"); console.log(" annotate --label <label> --name <name> --key <key> --value <value>"); console.log(""); console.log("Internal (normally run by review.js during Phase 1; here for manual re-runs):"); @@ -203,6 +207,12 @@ async function main() { case "addGrammarRule": result = await fn(g, args.name, args.production); break; + case "linkRule": + result = await fn(g, args.step, args.rule, args.confidence); + break; + case "mapCoverage": + result = await fn(g, args.test, args.file, args.step, args.confidence); + break; case "annotate": result = await fn(g, args.label, args.name, args.key, args.value); break; diff --git a/.skills/tinker-review/scripts/extraction/tree-sitter.js b/.skills/tinker-review/scripts/extraction/tree-sitter.js index 661a67ccc6..f674e63c31 100644 --- a/.skills/tinker-review/scripts/extraction/tree-sitter.js +++ b/.skills/tinker-review/scripts/extraction/tree-sitter.js @@ -20,7 +20,8 @@ import { createRequire } from "node:module"; const require = createRequire(import.meta.url); const TreeSitter = require("web-tree-sitter"); -import { readdir, readFile } from "node:fs/promises"; +import { readdir } from "node:fs/promises"; +import { readFileSync } from "node:fs"; import { join, relative } from "node:path"; import { fileURLToPath } from "node:url"; import { dirname } from "node:path"; @@ -305,7 +306,104 @@ function extractTypeName(node, language) { return nameNode ? nameNode.text : null; } -function extractTypesFromTree(tree, filePath, language) { +// Reduce a type-reference node (possibly generic or dotted) to its simple name, +// mirroring how calls resolve by simple name: `List<Foo>` -> "List", +// `a.b.Bar` -> "Bar". Returns null for shapes we can't name. +function baseTypeName(node) { + if (!node) return null; + if (node.type === "type_identifier" || node.type === "identifier") return node.text; + if (node.type === "scoped_type_identifier" || node.type === "qualified_name") { + let last = null; + for (let i = 0; i < node.childCount; i++) { + const c = node.child(i); + if (c.type === "type_identifier" || c.type === "identifier") last = c; + } + return last ? last.text : null; + } + if (node.type === "generic_type" || node.type === "generic_name") { + for (let i = 0; i < node.childCount; i++) { + const c = node.child(i); + const n = baseTypeName(c); + if (n) return n; + } + } + return null; +} + +const TYPE_REF_NODES = new Set([ + "type_identifier", "scoped_type_identifier", "generic_type", + "identifier", "qualified_name", "generic_name", +]); + +// Collect the simple names of every type reference directly under a container +// (e.g. a `super_interfaces`/`base_list`/`type_list` node). +function typeNamesInContainer(container) { + if (!container) return []; + const list = childByType(container, "type_list") || container; + const names = []; + for (let i = 0; i < list.childCount; i++) { + const child = list.child(i); + if (TYPE_REF_NODES.has(child.type)) { + const n = baseTypeName(child); + if (n) names.push(n); + } + } + return names; +} + +// Extract a type's declared supertypes as {name, relation} pairs, where relation +// is "extends" (superclass / interface-extends-interface) or "implements" +// (class-implements-interface). Java is split precisely; other languages emit a +// best-effort "extends" for every base since their grammars don't cleanly +// separate the two. Go has no inheritance, so it yields nothing. +function extractSupertypes(node, language) { + const supers = []; + const push = (names, relation) => { + for (const name of names) supers.push({ name, relation }); + }; + + if (language === "java") { + push(typeNamesInContainer(childByType(node, "superclass")), "extends"); + push(typeNamesInContainer(childByType(node, "super_interfaces")), "implements"); + // `interface A extends B` — an interface extending interfaces + push(typeNamesInContainer(childByType(node, "extends_interfaces")), "extends"); + return supers; + } + + if (language === "csharp") { + // `class C : Base, IFoo` — base_list mixes the superclass and interfaces + // with no grammatical distinction, so label them all "extends". + push(typeNamesInContainer(childByType(node, "base_list")), "extends"); + return supers; + } + + if (language === "javascript") { + const heritage = childByType(node, "class_heritage"); + if (heritage) { + const id = deepChildByType(heritage, "identifier"); + if (id) supers.push({ name: id.text, relation: "extends" }); + } + return supers; + } + + if (language === "python") { + const args = childByType(node, "argument_list"); + if (args) push(typeNamesInContainer(args), "extends"); + return supers; + } + + if (language === "dart") { + const sup = childByType(node, "superclass"); + if (sup) push(typeNamesInContainer(sup), "extends"); + const interfaces = childByType(node, "interfaces"); + if (interfaces) push(typeNamesInContainer(interfaces), "implements"); + return supers; + } + + return supers; +} + +function extractTypesFromTree(tree, filePath, language, fileChanged) { const types = []; function visit(node) { @@ -317,6 +415,8 @@ function extractTypesFromTree(tree, filePath, language) { kind: inferTypeKind(node, language), visibility: getVisibility(node, language), filePath, + changed: fileChanged, + supertypes: extractSupertypes(node, language), }); } } @@ -329,6 +429,29 @@ function extractTypesFromTree(tree, filePath, language) { return types; } +// Which type declares each function, for `declares` (Type -> Function) edges. +// Walks the tree carrying the innermost enclosing type so a method maps to the +// class/interface whose body it sits in (both keyed within the same file). +function extractDeclaresFromTree(tree, filePath, language) { + const declares = []; + + function visit(node, enclosingType) { + let currentType = enclosingType; + if (isTypeNode(node, language)) { + currentType = extractTypeName(node, language) || enclosingType; + } else if (isFunctionNode(node, language) && enclosingType) { + const fname = extractFunctionName(node, language); + if (fname) declares.push({ typeName: enclosingType, functionName: fname, filePath }); + } + for (let i = 0; i < node.childCount; i++) { + visit(node.child(i), currentType); + } + } + + visit(tree.rootNode, null); + return declares; +} + function extractCalleeName(node, language) { if (language === "java") { const nameNode = childByType(node, "identifier"); @@ -505,6 +628,147 @@ function classifyTestType(filePath, language) { return "unit"; } +// Parse one source file and append everything it yields to `result`. Shared by +// the changed-file pass and the hierarchy-neighborhood pass; `file.changed` +// flows onto every function/type so context files land as changed:false. +function parseSourceFile(parser, file, language, result) { + let content; + try { + content = readFileSync(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") return; + throw err; + } + const tree = parser.parse(content); + if (!tree) return; + + result.files.push({ path: file.path, language, changed: file.changed }); + + const fileFunctions = extractFunctionsFromTree(tree, file.path, language, file.changed); + result.functions.push(...fileFunctions); + result.types.push(...extractTypesFromTree(tree, file.path, language, file.changed)); + result.declares.push(...extractDeclaresFromTree(tree, file.path, language)); + + const fileCalls = extractCallsFromTree(tree, file.path, language, fileFunctions); + result.calls.push(...fileCalls); + result.imports.push(...extractImportsFromTree(tree, file.path, language)); + + if (isTestFile(file.path, language)) { + for (const fn of fileFunctions) { + result.tests.push({ + name: fn.name, + type: classifyTestType(file.path, language), + filePath: file.path, + calledFunctions: fileCalls.filter((c) => c.callerName === fn.name).map((c) => c.calleeName), + }); + } + } + + tree.delete(); +} + +// Approximate, regex-based scan of one file's text for type declarations and the +// type names they reference in a supertype position. Deliberately NOT tree-sitter: +// this only SELECTS which files the neighborhood pass should accurately parse, so +// over- or under-inclusion costs at most a few extra/missing parses — never a +// wrong edge. Go is skipped (no inheritance). +function scanTypeHeaders(text, language) { + const decls = []; + const refs = []; + if (language === "go") return { decls, refs }; + + const declRe = /\b(?:class|interface|enum|struct|trait|record)\s+([A-Za-z_]\w*)/g; + let m; + while ((m = declRe.exec(text)) !== null) { + decls.push(m[1]); + // Header = from the declared name to the body opener (`{`), bounded so a + // brace-less declaration (Python) can't swallow the whole file. + const rest = text.slice(m.index + m[0].length); + const brace = rest.indexOf("{"); + const nl = rest.indexOf("\n"); + let end = brace === -1 ? rest.length : brace; + if (language === "python" && nl !== -1) end = Math.min(end, nl); + end = Math.min(end, 300); + for (const r of rest.slice(0, end).match(/\b[A-Z]\w*/g) || []) { + if (r !== m[1]) refs.push(r); + } + } + return { decls, refs }; +} + +/** + * Find the type-hierarchy neighborhood of the changed types on disk: files that + * must also be parsed so override/hierarchy edges don't undercount. Two bounded + * BFS walks over a cheap regex index of the worktree: + * + * - Downward: files declaring a type that extends/implements a changed type + * (transitively) — the overriders an interface change actually affects. + * - Upward: files declaring a changed type's supertypes, to the root — so a + * changed subclass resolves its overrides against real ancestor methods + * instead of empty external stubs. + * + * Returns relative paths to parse as context (changed:false), capped so a change + * to a very central type can't drag in the whole module. + */ +async function hierarchyNeighborhood(directory, extensions, language, changedTypes, changedPaths, opts = {}) { + const maxDepth = opts.maxDepth ?? 6; + const maxFiles = opts.maxFiles ?? 250; + + const all = await walkDirectory(directory, extensions, new Set()); + const index = []; + for (const f of all) { + if (changedPaths.has(f.path)) continue; + let text; + try { text = readFileSync(f.fullPath, "utf-8"); } catch { continue; } + const { decls, refs } = scanTypeHeaders(text, language); + if (decls.length === 0) continue; + index.push({ path: f.path, fullPath: f.fullPath, decls: new Set(decls), refs: new Set(refs) }); + } + + const included = new Set(); + const include = (entry) => { + included.add(entry.path); + return included.size >= maxFiles; + }; + + // Downward: who subtypes a frontier type. + let downFrontier = new Set(changedTypes.map((t) => t.name)); + const downSeen = new Set(downFrontier); + for (let d = 0; d < maxDepth && downFrontier.size; d++) { + const next = new Set(); + for (const entry of index) { + if (included.has(entry.path)) continue; + let hit = false; + for (const r of entry.refs) if (downFrontier.has(r)) { hit = true; break; } + if (!hit) continue; + if (include(entry)) return { files: [...included], truncated: true }; + for (const dn of entry.decls) if (!downSeen.has(dn)) { downSeen.add(dn); next.add(dn); } + } + downFrontier = next; + } + + // Upward: declarers of a frontier supertype, following their own supertypes up. + let upFrontier = new Set(); + for (const t of changedTypes) for (const s of (t.supertypes || [])) upFrontier.add(s.name); + const upSeen = new Set(upFrontier); + for (let d = 0; d < maxDepth && upFrontier.size; d++) { + const next = new Set(); + for (const entry of index) { + if (included.has(entry.path)) continue; + let hit = false; + for (const dn of entry.decls) if (upFrontier.has(dn)) { hit = true; break; } + if (!hit) continue; + if (include(entry)) return { files: [...included], truncated: true }; + for (const r of entry.refs) if (!upSeen.has(r)) { upSeen.add(r); next.add(r); } + } + upFrontier = next; + } + + return { files: [...included], truncated: false }; +} + /** * Parse source files in a directory using Tree-sitter. * Returns structured extraction data for graph population. @@ -513,6 +777,9 @@ function classifyTestType(filePath, language) { * @param {string} language - Primary language to parse (e.g., "dart") * @param {object} options * @param {string[]} [options.changedFiles] - List of files changed in PR (relative paths) + * @param {boolean} [options.expandHierarchy] - Pull in the type-hierarchy + * neighborhood of changed types so override/hierarchy edges don't undercount + * (default: true, only applies in changed-files mode) * @returns {Promise<ExtractionResult>} */ export async function extract(directory, language, options = {}) { @@ -546,53 +813,29 @@ export async function extract(directory, language, options = {}) { calls: [], imports: [], tests: [], + declares: [], }; for (const file of sourceFiles) { - 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; + parseSourceFile(parser, file, language, result); + } + + // Hierarchy-neighborhood expansion: in changed-files mode the graph only holds + // PR-touched files, so a changed interface's overriders (and a changed + // subclass's ancestors) live in unparsed files — override/hierarchy edges + // would undercount. Pull those neighborhood files in as context (changed:false) + // so the edges resolve against real vertices instead of empty external stubs. + const expandHierarchy = options.expandHierarchy !== false; + if (expandHierarchy && changedFiles.length > 0 && result.types.length > 0) { + const changedPaths = new Set(sourceFiles.map((f) => f.path)); + const changedTypes = result.types.slice(); + const { files: extraPaths, truncated } = await hierarchyNeighborhood( + directory, extensions, language, changedTypes, changedPaths, + ); + for (const relPath of extraPaths) { + parseSourceFile(parser, { path: relPath, fullPath: join(directory, relPath), changed: false }, language, result); } - const tree = parser.parse(content); - if (!tree) continue; - - result.files.push({ - path: file.path, - language, - changed: file.changed, - }); - - const fileFunctions = extractFunctionsFromTree(tree, file.path, language, file.changed); - result.functions.push(...fileFunctions); - - const fileTypes = extractTypesFromTree(tree, file.path, language); - result.types.push(...fileTypes); - - const fileCalls = extractCallsFromTree(tree, file.path, language, fileFunctions); - result.calls.push(...fileCalls); - - const fileImports = extractImportsFromTree(tree, file.path, language); - result.imports.push(...fileImports); - - if (isTestFile(file.path, language)) { - for (const fn of fileFunctions) { - result.tests.push({ - name: fn.name, - type: classifyTestType(file.path, language), - filePath: file.path, - calledFunctions: fileCalls - .filter((c) => c.callerName === fn.name) - .map((c) => c.calleeName), - }); - } - } - - tree.delete(); + result.hierarchyNeighborhood = { files: extraPaths.length, truncated }; } parser.delete(); diff --git a/.skills/tinker-review/scripts/graph/populate.js b/.skills/tinker-review/scripts/graph/populate.js index bc06379fbe..8542af8d6b 100644 --- a/.skills/tinker-review/scripts/graph/populate.js +++ b/.skills/tinker-review/scripts/graph/populate.js @@ -22,7 +22,7 @@ import { join } from "node:path"; import gremlin from "gremlin"; import { CONFIDENCE } from "./confidence.js"; -const { process: { statics: __ } } = gremlin; +const { process: { statics: __, P } } = gremlin; const BATCH_SIZE = 50; @@ -31,6 +31,49 @@ async function submitBatch(batch) { return results.filter((r) => r.status === "fulfilled").length; } +/** + * Derive `overrides` edges (Function -> Function) from the type hierarchy. + * + * For each real Function `f` declared by some Type, walk that type's transitive + * supertypes (extends/implements) and, for every ancestor method sharing `f`'s + * name, add `f -overrides-> ancestorMethod`. This is what lets blast radius see + * impact flowing through interface/abstract hierarchies: change an interface + * method and every override is reachable via `in("overrides")`. + * + * Requires the `extends`/`implements` and `declares` edges to already be in the + * graph, so it runs as a final pass after population. INFERRED — the match is by + * method name (arity isn't reliably captured across languages). + * + * @param {object} g - gremlin-js GraphTraversalSource (already connected) + * @returns {Promise<number>} count of overrides edges created + */ +export async function deriveOverrides(g) { + // Only functions that have a declaring type can override anything. + const fnIds = await g.V().hasLabel("Function").hasNot("external") + .where(__.in_("declares")).id().toList(); + + let created = 0; + for (const fid of fnIds) { + // One scoped walk per function: a global dedup() here would dedup ancestor + // types ACROSS seeds, so a shared ancestor (e.g. a common interface) would + // be walked only once total and most overrides would be lost. Per-seed keeps + // each walk independent. + const result = await g.V(fid).as("f") + .in_("declares") + .repeat(__.out("extends", "implements")).emit().times(6) + .dedup() + .out("declares").hasNot("external") + .where(P.eq("f")).by("name") + .where(P.neq("f")) + .dedup() + .addE("overrides").from_("f") + .property("confidence", CONFIDENCE.INFERRED) + .count().next(); + created += Number(result.value); + } + return created; +} + /** * Populate TinkerGraph with extraction data. * Creates vertices and edges matching the PR knowledge graph schema. @@ -49,7 +92,7 @@ export async function populate(g, extraction, options = {}) { const counts = { vertices: 0, edges: 0, - breakdown: { files: 0, functions: 0, types: 0, tests: 0, calls: 0, defines: 0, testsEdges: 0, externalFunctions: 0, stubFiles: 0 }, + breakdown: { files: 0, functions: 0, types: 0, tests: 0, calls: 0, defines: 0, testsEdges: 0, externalFunctions: 0, stubFiles: 0, externalTypes: 0, extendsEdges: 0, implementsEdges: 0, declares: 0, overrides: 0 }, }; for (const file of extraction.files) { @@ -106,6 +149,7 @@ export async function populate(g, extraction, options = {}) { .property("kind", type.kind) .property("visibility", type.visibility) .property("filePath", type.filePath) + .property("changed", type.changed === undefined ? false : type.changed) .next(); counts.vertices++; counts.breakdown.types++; @@ -236,6 +280,73 @@ export async function populate(g, extraction, options = {}) { } } + // Resolve-or-mark supertypes. Like callees, a supertype is named in the source + // but its declaration is often outside the changed set (a JDK class, or a base + // type in a file this PR didn't touch). Materialize an external Type stub for + // any supertype name not among the extracted types so the extends/implements + // edge has a vertex to land on. Flushed before the edges below reference them. + const extractedTypeNames = new Set(extraction.types.map((t) => t.name)); + const unresolvedSupertypes = new Set(); + for (const type of extraction.types) { + for (const s of (type.supertypes || [])) { + if (!extractedTypeNames.has(s.name)) unresolvedSupertypes.add(s.name); + } + } + for (const name of unresolvedSupertypes) { + batch.push( + g.addV("Type") + .property("name", name) + .property("external", true) + .property("resolved", false) + ); + counts.breakdown.externalTypes++; + if (batch.length >= BATCH_SIZE) { + await submitBatch(batch); + batch = []; + } + } + if (batch.length > 0) { + await submitBatch(batch); + batch = []; + } + + // Type hierarchy edges (Type -> Type). INFERRED: the supertype is resolved by + // simple name, so the target is a deduction (like `calls`). `relation` is + // "extends" or "implements" as the source declared it. + for (const type of extraction.types) { + for (const s of (type.supertypes || [])) { + batch.push( + g.V().hasLabel("Type").has("name", type.name).has("filePath", type.filePath) + .addE(s.relation) + .property("confidence", CONFIDENCE.INFERRED) + .to(__.V().hasLabel("Type").has("name", s.name)) + ); + if (s.relation === "extends") counts.breakdown.extendsEdges++; + else counts.breakdown.implementsEdges++; + if (batch.length >= BATCH_SIZE) { + await submitBatch(batch); + batch = []; + } + } + } + + // Membership edges (Type -> Function). EXTRACTED: the method sits directly in + // the type's body — a directly observed fact, and both endpoints are pinned to + // the same file so the resolution is exact. + for (const decl of (extraction.declares || [])) { + batch.push( + g.V().hasLabel("Type").has("name", decl.typeName).has("filePath", decl.filePath) + .addE("declares") + .property("confidence", CONFIDENCE.EXTRACTED) + .to(__.V().hasLabel("Function").has("name", decl.functionName).has("filePath", decl.filePath)) + ); + counts.breakdown.declares++; + if (batch.length >= BATCH_SIZE) { + await submitBatch(batch); + batch = []; + } + } + // Import resolution (depends_on edges) is intentionally not implemented. // File-to-file connectivity is already captured through the calls/defines edges // (File A defines Function X which calls Function Y defined in File B). The @@ -245,8 +356,12 @@ export async function populate(g, extraction, options = {}) { if (batch.length > 0) { await submitBatch(batch); + batch = []; } + // Derive `overrides` edges now that the hierarchy and membership edges exist. + counts.breakdown.overrides = await deriveOverrides(g); + // 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 diff --git a/.skills/tinker-review/scripts/patterns/blast-radius.js b/.skills/tinker-review/scripts/patterns/blast-radius.js index 9e823e56bd..6dee31517b 100644 --- a/.skills/tinker-review/scripts/patterns/blast-radius.js +++ b/.skills/tinker-review/scripts/patterns/blast-radius.js @@ -26,10 +26,19 @@ const INHERENTLY_CENTRAL = new Set([ "finalize", "notify", "notifyAll", "wait", ]); +const { statics: __, t: T } = gremlin.process; + /** - * Calculate blast radius — how many functions are reachable downstream - * from changed functions via call edges. High blast radius means a change - * here affects many callers. + * Calculate blast radius — how far a change ripples. Two seeds: + * + * - Function-seeded: from each changed function, count what's reachable + * upstream via `calls` AND `overrides`. The `overrides` hop is what makes + * this see impact flowing through interface/abstract hierarchies — change an + * interface method and every override counts, which a call-only walk misses. + * - Type-seeded (hierarchy bucket): from each changed Type (typically an + * interface), count the functions declared by everything that implements or + * extends it. Reported separately so call/override impact and pure + * type-hierarchy impact stay legible rather than summed into one number. * * Methods that are inherently central (equals, toString, etc.) are filtered * out UNLESS they were modified in this PR. @@ -49,21 +58,29 @@ const INHERENTLY_CENTRAL = new Set([ * @property {number} linesStart * @property {number} linesEnd * @property {boolean} changed whether the PR modified this function - * @property {number} reachableCount callers reachable within `depth` hops upstream; high = the - * change ripples widely (for driver/server this is expected) + * @property {number} reachableCount callers + overriders reachable within `depth` hops upstream; + * high = the change ripples widely (for driver/server this is expected) * @property {number} depth hop limit used for this row * + * @typedef {Object} BlastRadiusType + * @property {string} name + * @property {string} filePath + * @property {string} kind class | interface | struct | enum + * @property {number} implementerCount functions declared by types that implement/extend this one + * @property {number} depth + * * @typedef {Object} BlastRadiusResult - * @property {BlastRadiusFn[]} functions changed functions and how far each one's change reaches - * @property {number} maxReachable largest reachableCount across changed functions - * @property {number} totalWithCallers changed functions that have any upstream callers - * @property {number} depth hop limit applied + * @property {BlastRadiusFn[]} functions changed functions and how far each one's change reaches + * @property {BlastRadiusType[]} types changed types and how many implementers' functions they reach + * @property {number} maxReachable largest reachableCount across changed functions + * @property {number} totalWithCallers changed functions that have any upstream callers/overriders + * @property {number} depth hop limit applied */ export async function blastRadius(g, params = {}) { const depth = params.depth || 3; const changedOnly = params.changedOnly !== false; - let traversal = g.V().hasLabel("Function"); + let traversal = g.V().hasLabel("Function").hasNot("external"); if (changedOnly) { traversal = traversal.has("changed", true); } @@ -72,14 +89,14 @@ export async function blastRadius(g, params = {}) { const results = []; for (const fnMap of functions) { - const vertexId = fnMap.get(gremlin.process.t.id); + const vertexId = fnMap.get(T.id); const name = fnMap.get("name"); const changed = fnMap.get("changed"); if (INHERENTLY_CENTRAL.has(name) && !changed) continue; const reachable = await g.V(vertexId) - .repeat(gremlin.process.statics.in_("calls")) + .repeat(__.union(__.in_("calls"), __.in_("overrides")).dedup()) .times(depth) .emit() .dedup() @@ -103,8 +120,46 @@ export async function blastRadius(g, params = {}) { results.sort((a, b) => b.reachableCount - a.reachableCount); + // Type-seeded hierarchy impact: functions declared by everything that + // implements/extends a changed type, walked in the implementer->supertype + // direction (`in`) up to `depth` levels of the hierarchy. + let typeTraversal = g.V().hasLabel("Type").hasNot("external"); + if (changedOnly) { + typeTraversal = typeTraversal.has("changed", true); + } + const changedTypes = await typeTraversal.elementMap().toList(); + const typeResults = []; + + for (const typeMap of changedTypes) { + const vertexId = typeMap.get(T.id); + const implementerCount = await g.V(vertexId) + .repeat(__.in_("implements", "extends").dedup()) + .times(depth) + .emit() + .dedup() + .out("declares") + .hasNot("external") + .dedup() + .count() + .next(); + + const count = Number(implementerCount.value); + if (count > 0) { + typeResults.push({ + name: typeMap.get("name"), + filePath: typeMap.get("filePath"), + kind: typeMap.get("kind"), + implementerCount: count, + depth, + }); + } + } + + typeResults.sort((a, b) => b.implementerCount - a.implementerCount); + return { functions: results, + types: typeResults, maxReachable: results.length > 0 ? results[0].reachableCount : 0, totalWithCallers: results.length, depth, diff --git a/.skills/tinker-review/scripts/renderer/render.js b/.skills/tinker-review/scripts/renderer/render.js index dbd71adbdb..6fdfce448e 100644 --- a/.skills/tinker-review/scripts/renderer/render.js +++ b/.skills/tinker-review/scripts/renderer/render.js @@ -403,6 +403,20 @@ function renderAppendixStructural(checks, graphStats) { return `<tr><td class="fn-name">${esc(b.name)}</td><td>${esc((b.filePath || "").split("/").pop())}</td><td class="num">${b.reachableCount}</td><td>${badge}</td></tr>`; }).join("\n "); + const hierarchy = checks?.blastRadius?.types || []; + const truncated = checks?.blastRadius?.neighborhood?.truncated; + const hierarchyRows = hierarchy.slice(0, 10).map(t => { + return `<tr><td class="fn-name">${esc(t.name)}</td><td>${esc(t.kind || "")}</td><td>${esc((t.filePath || "").split("/").pop())}</td><td class="num">${t.implementerCount}</td></tr>`; + }).join("\n "); + const truncNote = truncated ? ` <strong>Neighborhood truncated — these counts are a lower bound.</strong>` : ""; + const hierarchyHtml = hierarchy.length === 0 ? "" : ` + <h3>Type Hierarchy Impact</h3> + <p class="section-intro">For each changed type (typically an interface), the number of functions declared by everything that implements or extends it within 3 levels — impact that flows through the type hierarchy rather than direct calls.${truncNote}</p> + <table class="gap-table"> + <thead><tr><th>Type</th><th>Kind</th><th>File</th><th>Implementer fns</th></tr></thead> + <tbody>\n ${hierarchyRows}\n </tbody> + </table>`; + return `<section id="appendix-structural"> <h2>Appendix: Structural Data</h2> @@ -414,11 +428,12 @@ function renderAppendixStructural(checks, graphStats) { </table> <h3>Blast Radius</h3> - <p class="section-intro">Reachable callers within 3 hops upstream — higher means more code affected by behavioral changes.</p> + <p class="section-intro">Reachable callers and overriders within 3 hops upstream — higher means more code affected by behavioral changes. Includes impact flowing through interface/abstract overrides, not just direct calls.</p> <table class="gap-table"> <thead><tr><th>Function</th><th>File</th><th>Reachable</th><th></th></tr></thead> <tbody>\n ${blastRows}\n </tbody> </table> +${hierarchyHtml} ${confidenceHtml} <h3>Graph Statistics</h3> <div class="stats-grid"> diff --git a/.skills/tinker-review/scripts/review.js b/.skills/tinker-review/scripts/review.js index 848bbd4fde..bca8b44193 100644 --- a/.skills/tinker-review/scripts/review.js +++ b/.skills/tinker-review/scripts/review.js @@ -248,6 +248,10 @@ export async function phase1(session) { log(`Phase 1: Extracting structure (${language})...`); const extraction = await extract(worktreePath, language, { changedFiles }); log(`Phase 1 complete: ${extraction.files.length} files, ${extraction.functions.length} functions, ${extraction.types.length} types`); + const neighborhood = extraction.hierarchyNeighborhood; + if (neighborhood && neighborhood.files > 0) { + log(` hierarchy neighborhood: +${neighborhood.files} context files parsed for override/hierarchy edges${neighborhood.truncated ? " (TRUNCATED — hierarchy blast radius is a lower bound)" : ""}`); + } log(`Populating graph...`); const graphStats = await populate(g, extraction, { changedFiles, worktreePath }); @@ -315,13 +319,14 @@ export async function phase1(session) { const coverageResult = await coverageGaps(g, { changedOnly: true }); const centralityResult = await highCentrality(g, { changedOnly: true, topN: 10, minDegree: 3 }); const blastResult = await blastRadius(g, { depth: 3, changedOnly: true }); + blastResult.neighborhood = extraction.hierarchyNeighborhood || null; const clusterResult = await clusterAnalysis(a, { changedOnly: true }); const confidenceResult = await confidenceAudit(g); const orphansResult = await orphans(g, { vertexLabel: "Function", expectedEdge: "tests", direction: "in", changedOnly: true }); 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(` blast_radius: max ${blastResult.maxReachable} reachable, ${blastResult.types.length} changed types with hierarchy impact`); log(` clusters: ${clusterResult.clusterCount} (${clusterResult.coherent ? "coherent" : "fragmented"})`); log(` confidence: ${confidenceResult.distribution.EXTRACTED} extracted / ${confidenceResult.distribution.INFERRED} inferred / ${confidenceResult.distribution.AMBIGUOUS} ambiguous`); log(` orphans: ${orphansResult.totalOrphaned} functions with no test`); diff --git a/.skills/tinker-review/test/extraction.test.js b/.skills/tinker-review/test/extraction.test.js new file mode 100644 index 0000000000..db2c0dc3e3 --- /dev/null +++ b/.skills/tinker-review/test/extraction.test.js @@ -0,0 +1,151 @@ +/* + * 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. + */ + +// Verifies the tree-sitter extractor captures the type hierarchy: supertypes +// (split into extends/implements for Java) and the declares (Type -> method) +// membership relation that override derivation and type-seeded blast radius +// depend on. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtemp, writeFile, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +import { extract } from "../scripts/extraction/tree-sitter.js"; + +async function withJavaSources(files, fn) { + const dir = await mkdtemp(join(tmpdir(), "tinker-extract-")); + try { + const changed = []; + for (const [name, src] of Object.entries(files)) { + await writeFile(join(dir, name), src); + changed.push(name); + } + return await fn(await extract(dir, "java", { changedFiles: changed })); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +test("Java supertypes are split into extends and implements", async () => { + await withJavaSources( + { + "MapStep.java": + "package x;\npublic class MapStep extends AbstractStep implements Cloneable, Traversal {\n public Object next() { return null; }\n}\n", + }, + (r) => { + const t = r.types.find((t) => t.name === "MapStep"); + assert.ok(t, "MapStep type extracted"); + const supers = t.supertypes.reduce((m, s) => ((m[s.name] = s.relation), m), {}); + assert.equal(supers.AbstractStep, "extends"); + assert.equal(supers.Cloneable, "implements"); + assert.equal(supers.Traversal, "implements"); + }, + ); +}); + +test("an interface extending an interface uses the extends relation", async () => { + await withJavaSources( + { "Named.java": "package x;\ninterface Named extends Traversal { String name(); }\n" }, + (r) => { + const t = r.types.find((t) => t.name === "Named"); + assert.deepEqual(t.supertypes, [{ name: "Traversal", relation: "extends" }]); + }, + ); +}); + +test("declares maps each method to its enclosing type", async () => { + await withJavaSources( + { + "AbstractStep.java": + "package x;\nabstract class AbstractStep implements Traversal {\n public Object next() { return null; }\n protected void reset() {}\n}\n", + }, + (r) => { + const declared = r.declares + .filter((d) => d.typeName === "AbstractStep") + .map((d) => d.functionName) + .sort(); + assert.deepEqual(declared, ["next", "reset"]); + }, + ); +}); + +test("types carry the changed flag from their file", async () => { + await withJavaSources( + { "Traversal.java": "package x;\npublic interface Traversal { Object next(); }\n" }, + (r) => { + assert.equal(r.types.find((t) => t.name === "Traversal").changed, true); + }, + ); +}); + +// Hierarchy-neighborhood expansion (n6r): in changed-files mode the extractor +// pulls in the type-hierarchy neighborhood as context so override/hierarchy +// edges don't undercount. +const HIERARCHY = { + "Traversal.java": "package x;\npublic interface Traversal { Object next(); }\n", + "AbstractStep.java": + "package x;\nabstract class AbstractStep implements Traversal {\n public Object next() { return null; }\n}\n", + "MapStep.java": + "package x;\npublic class MapStep extends AbstractStep {\n public Object next() { return null; }\n}\n", + "Unrelated.java": "package x;\npublic class Unrelated { void foo() {} }\n", +}; + +async function withChanged(files, changedFiles, fn) { + const dir = await mkdtemp(join(tmpdir(), "tinker-hier-")); + try { + for (const [name, src] of Object.entries(files)) await writeFile(join(dir, name), src); + return await fn(await extract(dir, "java", { changedFiles })); + } finally { + await rm(dir, { recursive: true, force: true }); + } +} + +test("changing only an interface pulls its implementers in as context (downward)", async () => { + await withChanged(HIERARCHY, ["Traversal.java"], (r) => { + const byPath = r.files.reduce((m, f) => ((m[f.path] = f.changed), m), {}); + assert.equal(byPath["Traversal.java"], true); + assert.equal(byPath["AbstractStep.java"], false, "direct implementer pulled in"); + assert.equal(byPath["MapStep.java"], false, "transitive subtype pulled in"); + assert.equal(byPath["Unrelated.java"], undefined, "unrelated type not pulled in"); + }); +}); + +test("changing only a subclass pulls its ancestors in as context (upward)", async () => { + await withChanged(HIERARCHY, ["MapStep.java"], (r) => { + const byPath = r.files.reduce((m, f) => ((m[f.path] = f.changed), m), {}); + assert.equal(byPath["MapStep.java"], true); + assert.equal(byPath["AbstractStep.java"], false, "parent pulled in"); + assert.equal(byPath["Traversal.java"], false, "transitive ancestor pulled in"); + assert.equal(byPath["Unrelated.java"], undefined, "unrelated type not pulled in"); + }); +}); + +test("expandHierarchy:false keeps extraction to changed files only", async () => { + const dir = await mkdtemp(join(tmpdir(), "tinker-hier-")); + try { + for (const [name, src] of Object.entries(HIERARCHY)) await writeFile(join(dir, name), src); + const r = await extract(dir, "java", { changedFiles: ["Traversal.java"], expandHierarchy: false }); + assert.deepEqual(r.files.map((f) => f.path), ["Traversal.java"]); + assert.equal(r.hierarchyNeighborhood, undefined); + } finally { + await rm(dir, { recursive: true, force: true }); + } +});
