You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2026/07/15 12:25:40 UTC
(tinkerpop) branch master updated: Grade tinker-review changes as NONE/FORMATTING/BEHAVIORAL/STRUCTURAL
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
The following commit(s) were added to refs/heads/master by this push:
new 10a2306de6 Grade tinker-review changes as NONE/FORMATTING/BEHAVIORAL/STRUCTURAL
10a2306de6 is described below
commit 10a2306de6e29db6d56d219e1cb8bf221410e901
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Wed Jul 15 08:22:54 2026 -0400
Grade tinker-review changes as NONE/FORMATTING/BEHAVIORAL/STRUCTURAL
Replace the boolean `changed` flag with a `changeLevel` graded by diffing each
changed file's base version against PR-head. An untouched helper in a changed
file now grades NONE instead of polluting the risk checks; centrality,
blast-radius, and coverage-gaps default to the meaningful tiers (BEHAVIORAL,
STRUCTURAL) with an opt-in minChangeLevel. listFunctions gains --changeLevel;
--changed stays as an alias for "not NONE".
Assisted-by: Claude Code:claude-opus-4-8
---
.skills/tinker-review/references/enrichment-cli.md | 6 +-
.skills/tinker-review/references/interfaces.md | 38 +++--
.skills/tinker-review/references/schema.md | 52 ++++--
.skills/tinker-review/scripts/enrichment/api.js | 42 +++--
.skills/tinker-review/scripts/enrichment/cli.js | 4 +-
.../scripts/extraction/tree-sitter.js | 156 ++++++++++++++++--
.../tinker-review/scripts/graph/change-levels.js | 177 +++++++++++++++++++++
.skills/tinker-review/scripts/graph/populate.js | Bin 15295 -> 15392 bytes
.skills/tinker-review/scripts/graph/references.js | 2 +-
.../tinker-review/scripts/patterns/architecture.js | 14 +-
.../tinker-review/scripts/patterns/blast-radius.js | 27 +++-
.../tinker-review/scripts/patterns/centrality.js | 25 ++-
.../scripts/patterns/cluster-analysis.js | 4 +-
.../scripts/patterns/community-detection.js | 8 +-
.../scripts/patterns/coverage-gaps.js | 8 +-
.skills/tinker-review/scripts/patterns/orphans.js | 6 +-
.skills/tinker-review/scripts/renderer/render.js | 16 +-
.../tinker-review/scripts/renderer/template.html | 1 +
.skills/tinker-review/scripts/review.js | 29 +++-
.skills/tinker-review/test/extraction.test.js | 20 +--
.skills/tinker-review/test/fingerprint.test.js | 136 ++++++++++++++++
21 files changed, 671 insertions(+), 100 deletions(-)
diff --git a/.skills/tinker-review/references/enrichment-cli.md b/.skills/tinker-review/references/enrichment-cli.md
index 044108451e..e42247c603 100644
--- a/.skills/tinker-review/references/enrichment-cli.md
+++ b/.skills/tinker-review/references/enrichment-cli.md
@@ -55,8 +55,10 @@ before enriching and to pull the verification worklist.
### listFunctions
Lists functions the graph knows about. **Your first orientation read.**
-`--changed true` narrows to just what the PR touched; `--visibility public`
-narrows to the API surface. Returns each function's signature and line span so
+`--changeLevel STRUCTURAL` narrows to signature-level changes (also `BEHAVIORAL`,
+`FORMATTING`, `NONE`); `--changed true` is a back-compat alias for "anything but
+NONE" (and `--changed false` for NONE only); `--visibility public` narrows to the
+API surface. Each result carries its `changeLevel`, signature, and line span so
you can open it in the worktree. Reach for it at the top of almost any playbook.
### listTypes
diff --git a/.skills/tinker-review/references/interfaces.md b/.skills/tinker-review/references/interfaces.md
index 93f290c542..282e06adf1 100644
--- a/.skills/tinker-review/references/interfaces.md
+++ b/.skills/tinker-review/references/interfaces.md
@@ -21,20 +21,24 @@ interface ExtractionResult {
imports: ImportInfo[];
}
+// How much the PR moved a vertex, graded by diffing base against PR-head.
+// Vocabulary/helpers: scripts/graph/change-levels.js.
+type ChangeLevel = "NONE" | "FORMATTING" | "BEHAVIORAL" | "STRUCTURAL";
+
interface FileInfo {
- path: string; // relative to worktree root
- language: string; // e.g., "dart", "java", "go"
- changed: boolean; // true if modified in this PR
+ path: string; // relative to worktree root
+ language: string; // e.g., "dart", "java", "go"
+ changeLevel: ChangeLevel; // rollup of the file's members + import/export delta
}
interface FunctionInfo {
name: string;
- signature: string; // full signature as string
+ signature: string; // full signature as string
visibility: "public" | "private" | "protected" | "internal";
- filePath: string; // which file this lives in
+ filePath: string; // which file this lives in
linesStart: number;
linesEnd: number;
- changed: boolean; // true if modified in this PR
+ changeLevel: ChangeLevel; // graded per function (untouched helper = NONE)
}
interface TypeInfo {
@@ -42,6 +46,7 @@ interface TypeInfo {
kind: "class" | "interface" | "struct" | "enum";
visibility: "public" | "private" | "protected" | "internal";
filePath: string;
+ changeLevel: ChangeLevel; // graded over the type's declaration surface
}
interface CallInfo {
@@ -159,6 +164,9 @@ interface ReportPackage extends Evidence {
* @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 {Object<string,string>} [options.baseContents] - Base-version source of
+ * each changed file, keyed by path, so each member's `changeLevel` can be graded
+ * by diffing base against head. A missing entry = a file the PR added (STRUCTURAL).
* @returns {Promise<ExtractionResult>}
*/
export async function extract(directory, language, options = {}) {}
@@ -167,8 +175,8 @@ export async function extract(directory, language, options = {}) {}
**Responsibilities:**
- Load the appropriate tree-sitter grammar for the language
- Walk the directory, parse each source file
-- Run queries to extract functions, types, call sites, imports
-- Mark `changed: true` on files/functions that appear in `options.changedFiles`
+- Grade each file/function/type's `changeLevel` against `options.baseContents`
+ (context / hierarchy-neighborhood files, absent from `changedFiles`, grade `NONE`)
- Return the structured `ExtractionResult`
**Does NOT:**
@@ -232,9 +240,9 @@ export async function populate(g, extraction) {}
| Extraction data | Graph vertex | Key properties |
|---|---|---|
-| `files[]` | `File` | path, language, changed |
-| `functions[]` | `Function` | name, signature, visibility, lines_start, lines_end, changed |
-| `types[]` | `Type` | name, kind, visibility |
+| `files[]` | `File` | path, language, changeLevel |
+| `functions[]` | `Function` | name, signature, visibility, lines_start, lines_end, changeLevel |
+| `types[]` | `Type` | name, kind, visibility, changeLevel |
| Extraction data | Graph edge | From → To |
|---|---|---|
@@ -283,7 +291,8 @@ export async function completeness(g, params) {}
*
* @param {object} g - gremlin-js GraphTraversalSource
* @param {object} params
- * @param {boolean} [params.changedOnly] - Only check functions with changed=true (default: true)
+ * @param {boolean} [params.changedOnly] - Only check meaningfully-changed functions
+ * — BEHAVIORAL or STRUCTURAL (default: true)
* @returns {Promise<CoverageGapResult>}
*/
export async function coverageGaps(g, params = {}) {}
@@ -337,10 +346,11 @@ export async function review(params) {}
**Orchestration steps:**
1. `git fetch origin pull/${pr}/head:pr-review/${pr}`
2. `git worktree add /tmp/pr-review-${pr} pr-review/${pr}`
-3. Determine changed files via `git diff --name-only ${base}...pr-review/${pr}`
+3. Determine changed files via `git diff --name-only ${base}...pr-review/${pr}`, and
+ their base-version source via `git show ${base}:${path}` (for `changeLevel` grading)
4. `startServer()`
5. Connect gremlin-js to `handle.url`
-6. `extract(worktreePath, language, { changedFiles })`
+6. `extract(worktreePath, language, { changedFiles, baseContents })`
7. `populate(g, extraction)`
8. `completeness(g, { ... })`
9. `coverageGaps(g, { ... })`
diff --git a/.skills/tinker-review/references/schema.md b/.skills/tinker-review/references/schema.md
index 47df6e17a3..f80dbef8b4 100644
--- a/.skills/tinker-review/references/schema.md
+++ b/.skills/tinker-review/references/schema.md
@@ -4,24 +4,28 @@
### Code structure
-**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
+**File** `{ path, language, changeLevel }`
+A source file in the PR. `changeLevel` grades how much the PR moved it (see
+**Change levels** below); a file the PR touched is anything but `NONE`.
+
+*Stub Files* `{ path, language, changeLevel: "STRUCTURAL", parsed: false, deleted }`
+are markers for changed files the extractor didn't parse, so the PR's `modifies`
+edge still lands. Unparsed, so no fingerprint is possible — graded `STRUCTURAL`
+conservatively. `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, language, lines_start, lines_end, changed }`
-A function or method. The primary unit of analysis. `language` is the source
+**Function** `{ name, signature, visibility, filePath, language, lines_start, lines_end, changeLevel }`
+A function or method. The primary unit of analysis. `changeLevel` is graded per
+function by diffing the base against the PR-head version — so an untouched helper
+in a changed file grades `NONE`, not "changed". `language` is the source
language (java, python, javascript, go, csharp, dart); by-name edges (`calls`,
`tests`) resolve only within a language, so a multi-language PR never invents
cross-language edges.
-*External stub Functions* `{ name, external: true, resolved: false, changed: false, origin?, definedIn? }`
+*External stub Functions* `{ name, external: true, resolved: false, changeLevel: "NONE", origin?, definedIn? }`
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
@@ -35,10 +39,12 @@ 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, language, changed }`
+**Type** `{ name, kind, visibility, filePath, language, changeLevel }`
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. `language`
-scopes `extends`/`implements` resolution so hierarchies never cross languages.
+enum. `changeLevel` grades the type's declaration surface: a change to its
+kind/visibility/supertypes/member-set is `STRUCTURAL`; a change to method bodies
+in the declaration leaves it `BEHAVIORAL`/`FORMATTING`. `language` scopes
+`extends`/`implements` resolution so hierarchies never cross languages.
*External stub Types* `{ name, external: true, resolved: false }` are markers
created when an `extends`/`implements` edge names a supertype that wasn't
@@ -48,6 +54,28 @@ 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")`.
+#### Change levels
+
+`changeLevel` (on File, Function, Type) grades how much the PR moved a vertex,
+computed by diffing the base version of each changed file against PR-head. It
+replaces the old boolean `changed` — because that was stamped per file, it could
+not tell an untouched helper in a changed file from the code the PR rewrote.
+
+| Level | Meaning |
+|-------|---------|
+| `NONE` | byte-identical to base (untouched; also context / hierarchy-neighborhood files and external stubs) |
+| `FORMATTING` | only comments or whitespace moved; the tokens are identical |
+| `BEHAVIORAL` | the body changed for real, but the signature is stable |
+| `STRUCTURAL` | the signature/declaration changed, or the member/file was added or removed, or imports/exports changed |
+
+Query the two useful sets directly: **any real change** is
+`.has("changeLevel", within("FORMATTING","BEHAVIORAL","STRUCTURAL"))` (inclusion-
+oriented checks — orphans, cluster-analysis, architecture); **meaningful change**
+is `.has("changeLevel", within("BEHAVIORAL","STRUCTURAL"))` (risk checks —
+centrality, blast-radius, coverage-gaps, which discount NONE and FORMATTING by
+default). The vocabulary and predicate helpers live in
+`scripts/graph/change-levels.js`.
+
*Analysis-written property* — `community: number` is stamped onto Function, Type,
File and Test vertices by community detection (`communityDetection`, Louvain
modularity over the code subgraph). Vertices sharing a value are one densely-tied
diff --git a/.skills/tinker-review/scripts/enrichment/api.js b/.skills/tinker-review/scripts/enrichment/api.js
index c9b6c446d0..3f21cdf042 100644
--- a/.skills/tinker-review/scripts/enrichment/api.js
+++ b/.skills/tinker-review/scripts/enrichment/api.js
@@ -21,6 +21,7 @@ import { readFile } from "node:fs/promises";
import { join } from "node:path";
import gremlin from "gremlin";
import { CONFIDENCE, normalizeConfidence, isValidConfidence } from "../graph/confidence.js";
+import { changedAny, normalizeChangeLevel } from "../graph/change-levels.js";
import { symbolFromPath, createReferenceEdge } from "../graph/references.js";
const { process: { statics: __ } } = gremlin;
@@ -31,29 +32,40 @@ let cachedSteps = null;
/**
* List Function vertices, optionally filtered. The go-to orientation read:
- * `--changed true` shows just the functions this PR touched; `--visibility
- * public` narrows to the API surface. Returns signature and line span so you can
- * jump to source in the worktree.
+ * `--changeLevel STRUCTURAL` shows just the signature-level changes;
+ * `--changed true` (alias for "any level but NONE") shows everything the PR
+ * touched; `--visibility public` narrows to the API surface. Returns signature
+ * and line span so you can jump to source in the worktree.
*
* @param {object} g - gremlin-js GraphTraversalSource (already connected)
- * @param {object} [filter] - { changed?: boolean, visibility?: string, filePath?: string }
- * @returns {Promise<{name, signature, filePath, visibility, changed, linesStart, linesEnd}[]>}
+ * @param {object} [filter] - { changeLevel?: string, changed?: boolean, visibility?: string, filePath?: string }
+ * @returns {Promise<{name, signature, filePath, visibility, changeLevel, changed, linesStart, linesEnd}[]>}
*/
export async function listFunctions(g, filter = {}) {
let t = g.V().hasLabel("Function");
- if (filter.changed !== undefined) t = t.has("changed", filter.changed);
+ if (filter.changeLevel) {
+ // Exact-grade filter.
+ t = t.has("changeLevel", normalizeChangeLevel(filter.changeLevel, filter.changeLevel));
+ } else if (filter.changed !== undefined) {
+ // Back-compat alias: changed=true → anything but NONE; changed=false → NONE.
+ t = filter.changed ? t.has("changeLevel", changedAny()) : t.has("changeLevel", "NONE");
+ }
if (filter.visibility) t = t.has("visibility", filter.visibility);
if (filter.filePath) t = t.has("filePath", filter.filePath);
const results = await t.elementMap().toList();
- return results.map(m => ({
- name: m.get("name"),
- signature: m.get("signature"),
- filePath: m.get("filePath"),
- visibility: m.get("visibility"),
- changed: m.get("changed"),
- linesStart: m.get("lines_start"),
- linesEnd: m.get("lines_end"),
- }));
+ return results.map(m => {
+ const changeLevel = m.get("changeLevel");
+ return {
+ name: m.get("name"),
+ signature: m.get("signature"),
+ filePath: m.get("filePath"),
+ visibility: m.get("visibility"),
+ changeLevel,
+ changed: changeLevel !== undefined && changeLevel !== "NONE",
+ linesStart: m.get("lines_start"),
+ linesEnd: m.get("lines_end"),
+ };
+ });
}
/**
diff --git a/.skills/tinker-review/scripts/enrichment/cli.js b/.skills/tinker-review/scripts/enrichment/cli.js
index 989454d189..cac84dcc07 100644
--- a/.skills/tinker-review/scripts/enrichment/cli.js
+++ b/.skills/tinker-review/scripts/enrichment/cli.js
@@ -92,7 +92,7 @@ async function main() {
console.log(" node cli.js <command> --workDir /tmp/pr-review-3448 [--key value ...]");
console.log("");
console.log("Commands:");
- console.log(" listFunctions [--changed true] [--visibility public]");
+ console.log(" listFunctions [--changeLevel STRUCTURAL] [--changed true] [--visibility public]");
console.log(" listTypes [--kind class]");
console.log(" getCallsFrom --function <name> --file <path>");
console.log(" getCanonicalSteps");
@@ -150,7 +150,7 @@ async function main() {
switch (command) {
case "listFunctions":
- result = await fn(g, { changed: args.changed, visibility: args.visibility, filePath: args.file });
+ result = await fn(g, { changeLevel: args.changeLevel, changed: args.changed, visibility: args.visibility, filePath: args.file });
break;
case "listTypes":
result = await fn(g, { kind: args.kind, filePath: args.file });
diff --git a/.skills/tinker-review/scripts/extraction/tree-sitter.js b/.skills/tinker-review/scripts/extraction/tree-sitter.js
index 065f642020..8d425cdee9 100644
--- a/.skills/tinker-review/scripts/extraction/tree-sitter.js
+++ b/.skills/tinker-review/scripts/extraction/tree-sitter.js
@@ -22,9 +22,11 @@ const require = createRequire(import.meta.url);
const TreeSitter = require("web-tree-sitter");
import { readdir } from "node:fs/promises";
import { readFileSync } from "node:fs";
+import { createHash } from "node:crypto";
import { join, relative } from "node:path";
import { fileURLToPath } from "node:url";
import { dirname } from "node:path";
+import { CHANGE_LEVEL, gradeMember, gradeFile } from "../graph/change-levels.js";
const __filename = fileURLToPath(import.meta.url);
const __dirname = dirname(__filename);
@@ -248,7 +250,34 @@ function extractSignature(node, language) {
return `${name}${params}`;
}
-function extractFunctionsFromTree(tree, filePath, language, fileChanged) {
+function sha256(text) {
+ return createHash("sha256").update(text).digest("hex");
+}
+
+// Hash of a node's exact source text — distinguishes NONE (identical) from any
+// edit, including a pure reformat.
+function rawHash(node) {
+ return sha256(node.text);
+}
+
+// Hash of a node's leaf tokens with comment nodes skipped. tree-sitter does not
+// emit whitespace as tokens, so this is "code, minus formatting and comments":
+// two bodies with the same normHash differ only in whitespace/comments
+// (FORMATTING); differing normHash means the tokens themselves moved (BEHAVIORAL).
+function normHash(node) {
+ const tokens = [];
+ (function walk(n) {
+ if (n.type.includes("comment")) return;
+ if (n.childCount === 0) {
+ tokens.push(n.text);
+ return;
+ }
+ for (let i = 0; i < n.childCount; i++) walk(n.child(i));
+ })(node);
+ return sha256(tokens.join(" "));
+}
+
+function extractFunctionsFromTree(tree, filePath, language) {
const functions = [];
function visit(node) {
@@ -263,7 +292,8 @@ function extractFunctionsFromTree(tree, filePath, language, fileChanged) {
language,
linesStart: node.startPosition.row + 1,
linesEnd: node.endPosition.row + 1,
- changed: fileChanged,
+ rawHash: rawHash(node),
+ normHash: normHash(node),
});
}
}
@@ -404,21 +434,50 @@ function extractSupertypes(node, language) {
return supers;
}
-function extractTypesFromTree(tree, filePath, language, fileChanged) {
+// The function names declared anywhere inside a type node. Used only to build a
+// type's structural signature, so over-inclusion (e.g. a nested type's methods)
+// is harmless as long as it is consistent between the base and head parse.
+function declaredMemberNames(typeNode, language) {
+ const names = [];
+ (function walk(n) {
+ if (isFunctionNode(n, language)) {
+ const fname = extractFunctionName(n, language);
+ if (fname) names.push(fname);
+ }
+ for (let i = 0; i < n.childCount; i++) walk(n.child(i));
+ })(typeNode);
+ return names;
+}
+
+// A type's structural signature: its declaration surface. A change here (kind,
+// visibility, supertypes, or the set of declared members) is STRUCTURAL; a
+// change to method bodies only leaves it stable and grades FORMATTING/BEHAVIORAL.
+function typeSignature(node, language, kind, visibility, supertypes) {
+ const supers = supertypes.map((s) => `${s.relation}:${s.name}`).sort().join(",");
+ const members = declaredMemberNames(node, language).sort().join(",");
+ return `${kind}|${visibility}|extends[${supers}]|members[${members}]`;
+}
+
+function extractTypesFromTree(tree, filePath, language) {
const types = [];
function visit(node) {
if (isTypeNode(node, language)) {
const name = extractTypeName(node, language);
if (name) {
+ const kind = inferTypeKind(node, language);
+ const visibility = getVisibility(node, language);
+ const supertypes = extractSupertypes(node, language);
types.push({
name,
- kind: inferTypeKind(node, language),
- visibility: getVisibility(node, language),
+ kind,
+ visibility,
filePath,
language,
- changed: fileChanged,
- supertypes: extractSupertypes(node, language),
+ supertypes,
+ signature: typeSignature(node, language, kind, visibility, supertypes),
+ rawHash: rawHash(node),
+ normHash: normHash(node),
});
}
}
@@ -631,10 +690,43 @@ 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) {
+// A canonical key for a file's import set, so a change to what it imports (a
+// dependency-surface move) can be detected between base and head.
+function importSetKey(imports) {
+ return imports
+ .map((i) => `${i.importedPath || ""}::${i.importedName || ""}`)
+ .sort()
+ .join("|");
+}
+
+// Grade each head member against the base version, matching by name and
+// disambiguating overloads by signature. Returns levels aligned to `headMembers`.
+function gradeMembersAgainstBase(headMembers, baseMembers) {
+ const baseByName = new Map();
+ for (const b of baseMembers) {
+ if (!baseByName.has(b.name)) baseByName.set(b.name, []);
+ baseByName.get(b.name).push(b);
+ }
+ return headMembers.map((h) => {
+ const candidates = baseByName.get(h.name);
+ if (!candidates || candidates.length === 0) return CHANGE_LEVEL.STRUCTURAL;
+ // Prefer an exact-signature match; if the name is unique fall back to it so a
+ // signature change grades STRUCTURAL rather than "added". An ambiguous
+ // overload with no signature match means the overload set moved — STRUCTURAL.
+ let base = candidates.find((b) => b.signature === h.signature);
+ if (!base) base = candidates.length === 1 ? candidates[0] : null;
+ if (!base) return CHANGE_LEVEL.STRUCTURAL;
+ return gradeMember(base, h);
+ });
+}
+
+// Parse one source file and append everything it yields to `result`, stamping a
+// `changeLevel` on the file and each member. Shared by the changed-file pass and
+// the hierarchy-neighborhood pass; context files (`file.changed === false`) are
+// unchanged by the PR and grade NONE. A changed file is graded against its base
+// version from `baseContents`; a changed file with no base entry was added by
+// the PR and grades STRUCTURAL.
+function parseSourceFile(parser, file, language, result, baseContents = {}) {
let content;
try {
content = readFileSync(file.fullPath, "utf-8");
@@ -647,16 +739,47 @@ function parseSourceFile(parser, file, language, result) {
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);
+ const fileTypes = extractTypesFromTree(tree, file.path, language);
+ const fileImports = extractImportsFromTree(tree, file.path, language);
+
+ let fileLevel;
+ if (!file.changed) {
+ fileLevel = CHANGE_LEVEL.NONE;
+ for (const fn of fileFunctions) fn.changeLevel = CHANGE_LEVEL.NONE;
+ for (const t of fileTypes) t.changeLevel = CHANGE_LEVEL.NONE;
+ } else if (Object.prototype.hasOwnProperty.call(baseContents, file.path)) {
+ const baseContent = baseContents[file.path];
+ const baseTree = parser.parse(baseContent);
+ const baseFunctions = baseTree ? extractFunctionsFromTree(baseTree, file.path, language) : [];
+ const baseTypes = baseTree ? extractTypesFromTree(baseTree, file.path, language) : [];
+ const baseImports = baseTree ? extractImportsFromTree(baseTree, file.path, language) : [];
+ if (baseTree) baseTree.delete();
+
+ const fnLevels = gradeMembersAgainstBase(fileFunctions, baseFunctions);
+ fileFunctions.forEach((fn, i) => { fn.changeLevel = fnLevels[i]; });
+ const typeLevels = gradeMembersAgainstBase(fileTypes, baseTypes);
+ fileTypes.forEach((t, i) => { t.changeLevel = typeLevels[i]; });
+
+ fileLevel = gradeFile({
+ memberLevels: [...fnLevels, ...typeLevels],
+ importExportChanged: importSetKey(fileImports) !== importSetKey(baseImports),
+ rawFileChanged: sha256(content) !== sha256(baseContent),
+ });
+ } else {
+ fileLevel = CHANGE_LEVEL.STRUCTURAL;
+ for (const fn of fileFunctions) fn.changeLevel = CHANGE_LEVEL.STRUCTURAL;
+ for (const t of fileTypes) t.changeLevel = CHANGE_LEVEL.STRUCTURAL;
+ }
- const fileFunctions = extractFunctionsFromTree(tree, file.path, language, file.changed);
+ result.files.push({ path: file.path, language, changeLevel: fileLevel });
result.functions.push(...fileFunctions);
- result.types.push(...extractTypesFromTree(tree, file.path, language, file.changed));
+ result.types.push(...fileTypes);
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));
+ result.imports.push(...fileImports);
if (isTestFile(file.path, language)) {
for (const fn of fileFunctions) {
@@ -789,6 +912,7 @@ async function hierarchyNeighborhood(directory, extensions, language, changedTyp
export async function extract(directory, language, options = {}) {
const changedFiles = options.changedFiles || [];
const changedSet = new Set(changedFiles);
+ const baseContents = options.baseContents || {};
const extensions = LANGUAGE_EXTENSIONS[language];
if (!extensions) {
@@ -821,7 +945,7 @@ export async function extract(directory, language, options = {}) {
};
for (const file of sourceFiles) {
- parseSourceFile(parser, file, language, result);
+ parseSourceFile(parser, file, language, result, baseContents);
}
// Hierarchy-neighborhood expansion: in changed-files mode the graph only holds
diff --git a/.skills/tinker-review/scripts/graph/change-levels.js b/.skills/tinker-review/scripts/graph/change-levels.js
new file mode 100644
index 0000000000..97116f40b6
--- /dev/null
+++ b/.skills/tinker-review/scripts/graph/change-levels.js
@@ -0,0 +1,177 @@
+/*
+ * 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.
+ */
+
+/**
+ * Change-level vocabulary. Every File, Function and Type vertex carries a
+ * `changeLevel` recording how much the PR moved it, computed by diffing the base
+ * version of each changed file against the PR-head version. It replaces the old
+ * boolean `changed` flag, which — being stamped per file — could not tell an
+ * untouched helper in a changed file from the code the PR actually rewrote.
+ *
+ * NONE — byte-identical to the base (untouched; also context /
+ * hierarchy-neighborhood files and external stubs).
+ * FORMATTING — only comments or whitespace moved; the tokens are identical.
+ * BEHAVIORAL — the body changed for real, but the signature is stable.
+ * STRUCTURAL — the signature/declaration changed, or the member/file was
+ * added or removed, or imports/exports changed.
+ *
+ * Risk checks (centrality, blast-radius, coverage-gaps) default to the two
+ * meaningful tiers (BEHAVIORAL, STRUCTURAL); inclusion-oriented checks (orphans,
+ * cluster-analysis, architecture) count anything that is not NONE.
+ */
+
+import gremlin from "gremlin";
+
+const { process: { P } } = gremlin;
+
+export const CHANGE_LEVEL = Object.freeze({
+ NONE: "NONE",
+ FORMATTING: "FORMATTING",
+ BEHAVIORAL: "BEHAVIORAL",
+ STRUCTURAL: "STRUCTURAL",
+});
+
+// Ascending severity. Index in this array is the rank used to combine levels.
+export const ORDER = Object.freeze([
+ CHANGE_LEVEL.NONE,
+ CHANGE_LEVEL.FORMATTING,
+ CHANGE_LEVEL.BEHAVIORAL,
+ CHANGE_LEVEL.STRUCTURAL,
+]);
+
+const RANK = new Map(ORDER.map((level, i) => [level, i]));
+
+const VALID = new Set(ORDER);
+
+export function isValidChangeLevel(value) {
+ return VALID.has(value);
+}
+
+/**
+ * Normalize a caller-supplied change level (e.g. a `--changeLevel` flag),
+ * upper-casing and falling back when missing or invalid.
+ *
+ * @param {string|undefined} value
+ * @param {string|null} [fallback=null]
+ * @returns {string|null}
+ */
+export function normalizeChangeLevel(value, fallback = null) {
+ if (typeof value === "string") {
+ const upper = value.toUpperCase();
+ if (VALID.has(upper)) return upper;
+ }
+ return fallback;
+}
+
+/**
+ * The most severe of the given levels (NONE if none supplied). Used to roll a
+ * file's members up into a single File.changeLevel.
+ *
+ * @param {...string} levels
+ * @returns {string}
+ */
+export function maxLevel(...levels) {
+ let best = CHANGE_LEVEL.NONE;
+ for (const level of levels) {
+ if (!VALID.has(level)) continue;
+ if (RANK.get(level) > RANK.get(best)) best = level;
+ }
+ return best;
+}
+
+/**
+ * True if `level` is at least as severe as `floor`.
+ *
+ * @param {string} level
+ * @param {string} floor
+ * @returns {boolean}
+ */
+export function atLeast(level, floor) {
+ return VALID.has(level) && VALID.has(floor) && RANK.get(level) >= RANK.get(floor);
+}
+
+/**
+ * @typedef {Object} MemberFingerprint
+ * @property {string} signature the member's signature (name+params for a
+ * function; kind+visibility+supertypes+member-set
+ * for a type)
+ * @property {string} rawHash hash of the member's exact source text
+ * @property {string} normHash hash of the member's leaf tokens, comments removed
+ */
+
+/**
+ * Grade one head member against its matched base member. A missing base member
+ * (no match by name/signature) is a new member — STRUCTURAL.
+ *
+ * @param {MemberFingerprint|null|undefined} base
+ * @param {MemberFingerprint} head
+ * @returns {string} a CHANGE_LEVEL value
+ */
+export function gradeMember(base, head) {
+ if (!base) return CHANGE_LEVEL.STRUCTURAL; // added member
+ if (base.rawHash === head.rawHash) return CHANGE_LEVEL.NONE;
+ if (base.signature !== head.signature) return CHANGE_LEVEL.STRUCTURAL;
+ if (base.normHash === head.normHash) return CHANGE_LEVEL.FORMATTING;
+ return CHANGE_LEVEL.BEHAVIORAL;
+}
+
+/**
+ * Roll a file's members and its import/export delta into a single File level.
+ * An import/export set change is STRUCTURAL (a dependency-surface change). If the
+ * raw file bytes differ but every member is NONE and imports are stable, the
+ * change is top-level trivia outside any member — a FORMATTING floor.
+ *
+ * @param {object} params
+ * @param {string[]} params.memberLevels changeLevel of every Function/Type in the file
+ * @param {boolean} params.importExportChanged whether the import/export set moved
+ * @param {boolean} params.rawFileChanged whether the file's raw bytes differ from base
+ * @returns {string} a CHANGE_LEVEL value
+ */
+export function gradeFile({ memberLevels = [], importExportChanged = false, rawFileChanged = false }) {
+ let level = maxLevel(...memberLevels);
+ if (importExportChanged) level = maxLevel(level, CHANGE_LEVEL.STRUCTURAL);
+ if (level === CHANGE_LEVEL.NONE && rawFileChanged) return CHANGE_LEVEL.FORMATTING;
+ return level;
+}
+
+// === Gremlin predicate helpers ===
+// Keep the "what counts as changed" definition here so every check filters the
+// same way rather than open-coding `within(...)`.
+
+/** Any real change — everything but NONE. Inclusion-oriented checks use this. */
+export function changedAny() {
+ return P.within(CHANGE_LEVEL.FORMATTING, CHANGE_LEVEL.BEHAVIORAL, CHANGE_LEVEL.STRUCTURAL);
+}
+
+/** Meaningful change — BEHAVIORAL or STRUCTURAL. Risk checks default to this. */
+export function changedMeaningful() {
+ return P.within(CHANGE_LEVEL.BEHAVIORAL, CHANGE_LEVEL.STRUCTURAL);
+}
+
+/**
+ * A predicate matching every level at least as severe as `floor`. Backs the
+ * `minChangeLevel` opt-in on centrality/blast-radius.
+ *
+ * @param {string} floor - a CHANGE_LEVEL value
+ * @returns {object} a gremlin P predicate
+ */
+export function atLeastP(floor) {
+ const from = RANK.has(floor) ? RANK.get(floor) : 1;
+ return P.within(...ORDER.slice(from));
+}
diff --git a/.skills/tinker-review/scripts/graph/populate.js b/.skills/tinker-review/scripts/graph/populate.js
index 75bc915eeb..1a2dde4a87 100644
Binary files a/.skills/tinker-review/scripts/graph/populate.js and b/.skills/tinker-review/scripts/graph/populate.js differ
diff --git a/.skills/tinker-review/scripts/graph/references.js b/.skills/tinker-review/scripts/graph/references.js
index 21f84793bf..5494e6195e 100644
--- a/.skills/tinker-review/scripts/graph/references.js
+++ b/.skills/tinker-review/scripts/graph/references.js
@@ -69,7 +69,7 @@ export async function createReferenceEdge(g, params) {
await g.addV("File")
.property("path", fromPath)
.property("language", extOf(fromPath))
- .property("changed", false)
+ .property("changeLevel", "NONE")
.property("parsed", false)
.property("deleted", false)
.next();
diff --git a/.skills/tinker-review/scripts/patterns/architecture.js b/.skills/tinker-review/scripts/patterns/architecture.js
index 9c772d881d..d4ee27a061 100644
--- a/.skills/tinker-review/scripts/patterns/architecture.js
+++ b/.skills/tinker-review/scripts/patterns/architecture.js
@@ -18,6 +18,7 @@
*/
import gremlin from "gremlin";
+import { changedAny } from "../graph/change-levels.js";
const { process: { t } } = gremlin;
@@ -31,7 +32,8 @@ const { process: { t } } = gremlin;
* @param {object} g - gremlin-js GraphTraversalSource (already connected)
* @param {object} params
* @param {object} [params.clusterResult] - Output from clusterAnalysis() (connectedComponent clusters)
- * @param {boolean} [params.changedOnly] - Only include changed files (default: false)
+ * @param {boolean} [params.changedOnly] - Only include changed files — any level
+ * but NONE (default: false)
* @param {number} [params.maxNodes] - Cap on nodes to render (default: 40)
* @returns {Promise<ArchitectureResult>}
*/
@@ -41,7 +43,8 @@ const { process: { t } } = gremlin;
* @property {string} id
* @property {string} label
* @property {string} cluster the cluster/community the node belongs to
- * @property {boolean} changed whether the PR modified it
+ * @property {boolean} changed whether the PR modified it (changeLevel !== NONE)
+ * @property {string} changeLevel how the PR moved it (NONE | FORMATTING | BEHAVIORAL | STRUCTURAL)
*
* @typedef {Object} ArchitectureResult
* @property {ArchitectureNode[]} nodes
@@ -52,7 +55,7 @@ export async function architecture(g, params = {}) {
let traversal = g.V().hasLabel("File");
if (changedOnly) {
- traversal = traversal.has("changed", true);
+ traversal = traversal.has("changeLevel", changedAny());
}
const fileVertices = await traversal.elementMap().toList();
@@ -65,12 +68,13 @@ export async function architecture(g, params = {}) {
let nodes = fileVertices.map((fileMap) => {
const path = fileMap.get("path");
- const changed = fileMap.get("changed") || false;
+ const changeLevel = fileMap.get("changeLevel") || "NONE";
+ const changed = changeLevel !== "NONE";
const id = path;
const label = shortLabel(path);
const cluster = clusterAssignment.get(path) || dirCluster(path);
- return { id, label, cluster, changed };
+ return { id, label, cluster, changed, changeLevel };
});
if (nodes.length > maxNodes) {
diff --git a/.skills/tinker-review/scripts/patterns/blast-radius.js b/.skills/tinker-review/scripts/patterns/blast-radius.js
index 6dee31517b..319de22b3a 100644
--- a/.skills/tinker-review/scripts/patterns/blast-radius.js
+++ b/.skills/tinker-review/scripts/patterns/blast-radius.js
@@ -19,6 +19,8 @@
import gremlin from "gremlin";
+import { changedMeaningful, atLeastP, atLeast } from "../graph/change-levels.js";
+
const INHERENTLY_CENTRAL = new Set([
"equals", "hashCode", "toString", "clone", "close", "compareTo",
"iterator", "hasNext", "next", "get", "set", "size", "isEmpty",
@@ -46,7 +48,11 @@ const { statics: __, t: T } = gremlin.process;
* @param {object} g - gremlin-js GraphTraversalSource
* @param {object} params
* @param {number} [params.depth] - Max hops to traverse (default: 3)
- * @param {boolean} [params.changedOnly] - Start from changed functions only (default: true)
+ * @param {boolean} [params.changedOnly] - Start from changed functions only (default: true).
+ * Function seed defaults to the meaningful tiers (BEHAVIORAL, STRUCTURAL); the
+ * type seed is STRUCTURAL-only.
+ * @param {string} [params.minChangeLevel] - Narrow the function seed to this
+ * level and above (e.g. "STRUCTURAL")
* @returns {Promise<BlastRadiusResult>}
*/
@@ -57,7 +63,7 @@ const { statics: __, t: T } = gremlin.process;
* @property {string} signature
* @property {number} linesStart
* @property {number} linesEnd
- * @property {boolean} changed whether the PR modified this function
+ * @property {string} changeLevel how the PR moved this function (NONE | FORMATTING | BEHAVIORAL | STRUCTURAL)
* @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
@@ -79,10 +85,14 @@ const { statics: __, t: T } = gremlin.process;
export async function blastRadius(g, params = {}) {
const depth = params.depth || 3;
const changedOnly = params.changedOnly !== false;
+ // Function seed uses the meaningful tiers; `minChangeLevel` can narrow further.
+ const changePredicate = params.minChangeLevel
+ ? atLeastP(params.minChangeLevel)
+ : changedMeaningful();
let traversal = g.V().hasLabel("Function").hasNot("external");
if (changedOnly) {
- traversal = traversal.has("changed", true);
+ traversal = traversal.has("changeLevel", changePredicate);
}
const functions = await traversal.elementMap().toList();
@@ -91,9 +101,9 @@ export async function blastRadius(g, params = {}) {
for (const fnMap of functions) {
const vertexId = fnMap.get(T.id);
const name = fnMap.get("name");
- const changed = fnMap.get("changed");
+ const changeLevel = fnMap.get("changeLevel");
- if (INHERENTLY_CENTRAL.has(name) && !changed) continue;
+ if (INHERENTLY_CENTRAL.has(name) && !atLeast(changeLevel || "NONE", "BEHAVIORAL")) continue;
const reachable = await g.V(vertexId)
.repeat(__.union(__.in_("calls"), __.in_("overrides")).dedup())
@@ -111,7 +121,7 @@ export async function blastRadius(g, params = {}) {
signature: fnMap.get("signature"),
linesStart: fnMap.get("lines_start"),
linesEnd: fnMap.get("lines_end"),
- changed,
+ changeLevel,
reachableCount: count,
depth,
});
@@ -123,9 +133,12 @@ export async function blastRadius(g, params = {}) {
// 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.
+ // Type seed is STRUCTURAL-only: a hierarchy ripples through implementers only
+ // when the type's contract (kind/supertypes/member set) moved — a body-only
+ // (BEHAVIORAL) or formatting change to the declaring file does not.
let typeTraversal = g.V().hasLabel("Type").hasNot("external");
if (changedOnly) {
- typeTraversal = typeTraversal.has("changed", true);
+ typeTraversal = typeTraversal.has("changeLevel", "STRUCTURAL");
}
const changedTypes = await typeTraversal.elementMap().toList();
const typeResults = [];
diff --git a/.skills/tinker-review/scripts/patterns/centrality.js b/.skills/tinker-review/scripts/patterns/centrality.js
index 2e67be304c..ff0379a25c 100644
--- a/.skills/tinker-review/scripts/patterns/centrality.js
+++ b/.skills/tinker-review/scripts/patterns/centrality.js
@@ -18,6 +18,7 @@
*/
import gremlin from "gremlin";
+import { changedMeaningful, atLeastP, atLeast } from "../graph/change-levels.js";
const { process: { statics: __ } } = gremlin;
@@ -38,7 +39,11 @@ const INHERENTLY_CENTRAL = new Set([
*
* @param {object} g - gremlin-js GraphTraversalSource
* @param {object} params
- * @param {boolean} [params.changedOnly] - Only check changed functions (default: true)
+ * @param {boolean} [params.changedOnly] - Only check changed functions (default: true).
+ * "Changed" defaults to the meaningful tiers (BEHAVIORAL, STRUCTURAL); NONE and
+ * FORMATTING are excluded.
+ * @param {string} [params.minChangeLevel] - Narrow the changed filter to this
+ * level and above (e.g. "STRUCTURAL" for signature churn only)
* @param {number} [params.topN] - Return top N results (default: 10)
* @param {number} [params.minDegree] - Minimum combined in+out degree to include (default: 3)
* @param {boolean} [params.excludeLibrary] - Drop calls to library-origin external
@@ -54,7 +59,7 @@ const INHERENTLY_CENTRAL = new Set([
* @property {string} signature
* @property {number} linesStart
* @property {number} linesEnd
- * @property {boolean} changed whether the PR modified this function
+ * @property {string} changeLevel how the PR moved this function (NONE | FORMATTING | BEHAVIORAL | STRUCTURAL)
* @property {number} inDegree incoming call edges (how many functions call it)
* @property {number} outDegree outgoing call edges, excluding origin:library targets
* @property {number} totalDegree inDegree + outDegree — the centrality score
@@ -72,10 +77,15 @@ export async function highCentrality(g, params = {}) {
const topN = params.topN || 10;
const minDegree = params.minDegree || 3;
const excludeLibrary = params.excludeLibrary !== false;
+ // Default risk lens: BEHAVIORAL + STRUCTURAL. `minChangeLevel` can narrow to
+ // STRUCTURAL-only (signature churn) when a reviewer wants just the API surface.
+ const changePredicate = params.minChangeLevel
+ ? atLeastP(params.minChangeLevel)
+ : changedMeaningful();
let traversal = g.V().hasLabel("Function");
if (changedOnly) {
- traversal = traversal.has("changed", true);
+ traversal = traversal.has("changeLevel", changePredicate);
}
const functions = await traversal.elementMap().toList();
@@ -85,7 +95,10 @@ export async function highCentrality(g, params = {}) {
for (const fnMap of functions) {
const vertexId = fnMap.get(gremlin.process.t.id);
const name = fnMap.get("name");
- const changed = fnMap.get("changed");
+ const changeLevel = fnMap.get("changeLevel");
+ // A boilerplate method (equals/toString/…) is surfaced only when the PR
+ // changed it meaningfully — a formatting-only touch is not enough.
+ const meaningfullyChanged = atLeast(changeLevel || "NONE", "BEHAVIORAL");
const inDegree = await g.V(vertexId).inE("calls").count().next();
// Out-degree optionally skips calls to library-origin externals (getName,
@@ -108,14 +121,14 @@ export async function highCentrality(g, params = {}) {
signature: fnMap.get("signature"),
linesStart: fnMap.get("lines_start"),
linesEnd: fnMap.get("lines_end"),
- changed,
+ changeLevel,
inDegree: inCount,
outDegree: outCount,
totalDegree,
inherentlyCentral: INHERENTLY_CENTRAL.has(name),
};
- if (INHERENTLY_CENTRAL.has(name) && !changed) {
+ if (INHERENTLY_CENTRAL.has(name) && !meaningfullyChanged) {
filtered.push(entry);
} else {
results.push(entry);
diff --git a/.skills/tinker-review/scripts/patterns/cluster-analysis.js b/.skills/tinker-review/scripts/patterns/cluster-analysis.js
index 2017e3d69a..6c7737c868 100644
--- a/.skills/tinker-review/scripts/patterns/cluster-analysis.js
+++ b/.skills/tinker-review/scripts/patterns/cluster-analysis.js
@@ -17,6 +17,8 @@
* under the License.
*/
+import { changedAny } from "../graph/change-levels.js";
+
/**
* Determine whether a PR's changed files form one coherent change or multiple
* disconnected clusters. Uses connectedComponent() via the OLAP traversal source.
@@ -47,7 +49,7 @@ export async function clusterAnalysis(a, params = {}) {
let traversal = a.V().hasLabel("File");
if (changedOnly) {
- traversal = traversal.has("changed", true);
+ traversal = traversal.has("changeLevel", changedAny());
}
const results = await traversal
diff --git a/.skills/tinker-review/scripts/patterns/community-detection.js b/.skills/tinker-review/scripts/patterns/community-detection.js
index 4596618c60..51bc9d8d02 100644
--- a/.skills/tinker-review/scripts/patterns/community-detection.js
+++ b/.skills/tinker-review/scripts/patterns/community-detection.js
@@ -19,6 +19,7 @@
import Graph from "graphology";
import louvain from "graphology-communities-louvain";
+import { atLeast } from "../graph/change-levels.js";
/**
* Louvain modularity community detection over the PR's *code* subgraph.
@@ -68,7 +69,7 @@ const EDGE_LABELS = ["calls", "defines", "declares", "extends", "implements", "o
* @property {string[]} files files this community touches (rolled up), largest share first
* @property {string} dominantFile the file contributing the most members
* @property {Object} labelCounts member count per vertex label (Function/Type/File/Test)
- * @property {number} changedCount members with changed:true
+ * @property {number} changedCount members meaningfully changed (BEHAVIORAL or STRUCTURAL)
* @property {string} dominantLabel most common vertex label
* @property {number} testShare fraction of members living in test files — drives the role
* @property {?Object} churn `{ added, removed, mode }` for the community's files, when churn is supplied
@@ -108,7 +109,10 @@ async function extractCodeSubgraph(g) {
const nodes = vRaw.map((m) => {
const o = mapToObj(m);
- return { id: String(o.id), label: o.label, name: o.name, filePath: o.filePath || o.path, changed: o.changed === true };
+ // "changed" here means a meaningful change (BEHAVIORAL/STRUCTURAL); the
+ // changed-share reading distinguishes real change from context, and a
+ // formatting-only touch is context for that purpose.
+ return { id: String(o.id), label: o.label, name: o.name, filePath: o.filePath || o.path, changed: atLeast(o.changeLevel || "NONE", "BEHAVIORAL") };
});
const edges = eRaw.map((m) => {
diff --git a/.skills/tinker-review/scripts/patterns/coverage-gaps.js b/.skills/tinker-review/scripts/patterns/coverage-gaps.js
index 20cee1b7de..7cbd763665 100644
--- a/.skills/tinker-review/scripts/patterns/coverage-gaps.js
+++ b/.skills/tinker-review/scripts/patterns/coverage-gaps.js
@@ -18,13 +18,15 @@
*/
import gremlin from "gremlin";
+import { changedMeaningful } from "../graph/change-levels.js";
/**
* Find changed functions that have no incoming 'tests' edge.
*
* @param {object} g - gremlin-js GraphTraversalSource
* @param {object} params
- * @param {boolean} [params.changedOnly] - Only check functions with changed=true (default: true)
+ * @param {boolean} [params.changedOnly] - Only check meaningfully-changed functions
+ * — BEHAVIORAL or STRUCTURAL (default: true)
* @returns {Promise<CoverageGapResult>}
*/
@@ -46,7 +48,9 @@ export async function coverageGaps(g, params = {}) {
let traversal = g.V().hasLabel("Function");
if (changedOnly) {
- traversal = traversal.has("changed", true);
+ // A formatting-only change does not create a coverage gap; a real body or
+ // signature change without a test does.
+ traversal = traversal.has("changeLevel", changedMeaningful());
}
const functions = await traversal.elementMap().toList();
diff --git a/.skills/tinker-review/scripts/patterns/orphans.js b/.skills/tinker-review/scripts/patterns/orphans.js
index b9ef966dd1..15bf96b07c 100644
--- a/.skills/tinker-review/scripts/patterns/orphans.js
+++ b/.skills/tinker-review/scripts/patterns/orphans.js
@@ -18,6 +18,7 @@
*/
import gremlin from "gremlin";
+import { changedAny } from "../graph/change-levels.js";
/**
* Find orphan vertices — nodes that are missing expected relationships.
@@ -29,7 +30,8 @@ import gremlin from "gremlin";
* @param {string} params.vertexLabel - Label to check (e.g., "Step", "Function")
* @param {string} params.expectedEdge - Edge label that should exist
* @param {string} [params.direction] - "in" or "out" (default: "in")
- * @param {boolean} [params.changedOnly] - Only check changed vertices (default: false)
+ * @param {boolean} [params.changedOnly] - Only check changed vertices — any level
+ * but NONE (default: false)
* @returns {Promise<OrphanResult>}
*/
@@ -52,7 +54,7 @@ export async function orphans(g, params) {
let traversal = g.V().hasLabel(vertexLabel);
if (changedOnly) {
- traversal = traversal.has("changed", true);
+ traversal = traversal.has("changeLevel", changedAny());
}
const vertices = await traversal.elementMap().toList();
diff --git a/.skills/tinker-review/scripts/renderer/render.js b/.skills/tinker-review/scripts/renderer/render.js
index 3fadcb096e..6b4e3fe36f 100644
--- a/.skills/tinker-review/scripts/renderer/render.js
+++ b/.skills/tinker-review/scripts/renderer/render.js
@@ -32,6 +32,18 @@ function esc(str) {
.replace(/"/g, """);
}
+// Render a function/type's change level as a badge. STRUCTURAL (signature churn)
+// reads loudest; BEHAVIORAL is a body change; FORMATTING is muted; NONE shows
+// nothing.
+function changeLevelBadge(level) {
+ switch (level) {
+ case "STRUCTURAL": return `<span class="badge badge-structural">structural</span>`;
+ case "BEHAVIORAL": return `<span class="badge badge-modified">behavioral</span>`;
+ case "FORMATTING": return `<span class="badge badge-formatting">formatting</span>`;
+ default: return "";
+ }
+}
+
/**
* Fields typed as raw *code* (appendixFunctional.testCode / .fullOutput) are
* wrapped by the renderer in its own `<pre><code>` and escaped. Agents
@@ -498,12 +510,12 @@ function renderAppendixStructural(checks, graphStats) {
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>` : "";
+ const badge = changeLevelBadge(h.changeLevel);
return `<tr><td class="fn-name">${esc(h.name)}</td><td>${esc((h.filePath || "").split("/").pop())}</td><td class="num">${h.inDegree}</td><td class="num">${h.outDegree}</td><td class="num"><strong>${h.totalDegree}</strong></td><td>${badge}</td></tr>`;
}).join("\n ");
const blastRows = blast.slice(0, 10).map(b => {
- const badge = b.changed ? `<span class="badge badge-modified">modified</span>` : "";
+ const badge = changeLevelBadge(b.changeLevel);
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 ");
diff --git a/.skills/tinker-review/scripts/renderer/template.html b/.skills/tinker-review/scripts/renderer/template.html
index 42e12cd79d..53901292c2 100644
--- a/.skills/tinker-review/scripts/renderer/template.html
+++ b/.skills/tinker-review/scripts/renderer/template.html
@@ -35,6 +35,7 @@ header .meta { display: flex; gap: 1rem; flex-wrap: wrap; font-size: 0.875rem; c
.badge-domain { background: var(--primary); color: #fff; } .badge-attention { background: var(--danger); color: #fff; }
.badge-safe { background: var(--success); color: #fff; } .badge-context { background: #6f42c1; color: #fff; }
.badge-test { background: #20c997; color: #fff; } .badge-modified { background: var(--warning); color: #000; }
+.badge-structural { background: var(--danger); color: #fff; } .badge-formatting { background: #adb5bd; color: #000; }
nav { background: var(--bg-alt); border: 1px solid var(--border); border-radius: var(--radius); padding: 1rem; margin-bottom: 2rem; }
nav h2 { font-size: 0.875rem; text-transform: uppercase; letter-spacing: 0.05em; color: var(--text-muted); margin-bottom: 0.5rem; }
nav ul { list-style: none; display: flex; flex-wrap: wrap; gap: 0.5rem 1.5rem; } nav ul li a { font-size: 0.875rem; }
diff --git a/.skills/tinker-review/scripts/review.js b/.skills/tinker-review/scripts/review.js
index badcf72939..1e1df75f07 100644
--- a/.skills/tinker-review/scripts/review.js
+++ b/.skills/tinker-review/scripts/review.js
@@ -87,6 +87,32 @@ async function getChangedFiles(repoPath, prBranch, remote = "upstream", baseBran
return diffOutput.trim().split("\n").filter(Boolean);
}
+/**
+ * The base-version source text of each changed file, keyed by path, so extraction
+ * can diff base against head and grade every member's `changeLevel`. Reads
+ * `git show <merge-base>:<path>`; a path git can't produce — added by the PR, or
+ * binary — is omitted, and extraction treats a missing entry as an added file
+ * (STRUCTURAL). Returns `{ [path]: content }`.
+ */
+export async function getBaseContents(repoPath, prBranch, changedFiles, remote = "upstream", baseBranch = "master") {
+ const { stdout: baseCommit } = await exec(
+ "git", ["merge-base", prBranch, `${remote}/${baseBranch}`], { cwd: repoPath }
+ );
+ const base = baseCommit.trim();
+ const contents = {};
+ await Promise.all(changedFiles.map(async (path) => {
+ try {
+ const { stdout } = await exec(
+ "git", ["show", `${base}:${path}`], { cwd: repoPath, maxBuffer: 32 * 1024 * 1024 }
+ );
+ contents[path] = stdout;
+ } catch {
+ // Absent at base (added by the PR) or unreadable (binary) — leave unset.
+ }
+ }));
+ return contents;
+}
+
/**
* Per-file line churn and deletion status for the PR, so downstream analysis can
* describe *how* code changed (reduced vs. expanded), not just that it changed.
@@ -322,7 +348,8 @@ export async function phase1(session) {
const languages = session.languages || [language];
log(`Phase 1: Extracting structure (${languages.join("+")})...`);
- const extraction = await extractMulti(worktreePath, languages, { changedFiles });
+ const baseContents = await getBaseContents(repoPath, prBranch, changedFiles, remote, baseBranch);
+ const extraction = await extractMulti(worktreePath, languages, { changedFiles, baseContents });
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) {
diff --git a/.skills/tinker-review/test/extraction.test.js b/.skills/tinker-review/test/extraction.test.js
index dafa6bd6c9..cde766ba73 100644
--- a/.skills/tinker-review/test/extraction.test.js
+++ b/.skills/tinker-review/test/extraction.test.js
@@ -87,11 +87,11 @@ test("declares maps each method to its enclosing type", async () => {
);
});
-test("types carry the changed flag from their file", async () => {
+test("a changed file with no base version grades its members STRUCTURAL (added 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);
+ assert.equal(r.types.find((t) => t.name === "Traversal").changeLevel, "STRUCTURAL");
},
);
});
@@ -120,20 +120,20 @@ async function withChanged(files, changedFiles, fn) {
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");
+ const byPath = r.files.reduce((m, f) => ((m[f.path] = f.changeLevel), m), {});
+ assert.equal(byPath["Traversal.java"], "STRUCTURAL");
+ assert.equal(byPath["AbstractStep.java"], "NONE", "direct implementer pulled in as context");
+ assert.equal(byPath["MapStep.java"], "NONE", "transitive subtype pulled in as context");
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");
+ const byPath = r.files.reduce((m, f) => ((m[f.path] = f.changeLevel), m), {});
+ assert.equal(byPath["MapStep.java"], "STRUCTURAL");
+ assert.equal(byPath["AbstractStep.java"], "NONE", "parent pulled in as context");
+ assert.equal(byPath["Traversal.java"], "NONE", "transitive ancestor pulled in as context");
assert.equal(byPath["Unrelated.java"], undefined, "unrelated type not pulled in");
});
});
diff --git a/.skills/tinker-review/test/fingerprint.test.js b/.skills/tinker-review/test/fingerprint.test.js
new file mode 100644
index 0000000000..9909445b56
--- /dev/null
+++ b/.skills/tinker-review/test/fingerprint.test.js
@@ -0,0 +1,136 @@
+/*
+ * 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 structural fingerprinting: the pure grade logic (gradeMember /
+// gradeFile / maxLevel / atLeast) and the end-to-end grading in the tree-sitter
+// extractor, which diffs each changed file's base version against head and
+// stamps NONE / FORMATTING / BEHAVIORAL / STRUCTURAL.
+
+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";
+import { gradeMember, gradeFile, maxLevel, atLeast, CHANGE_LEVEL } from "../scripts/graph/change-levels.js";
+
+// === Pure grade logic ===
+
+test("gradeMember covers all four outcomes", () => {
+ const base = { signature: "f(a)", rawHash: "r1", normHash: "n1" };
+ assert.equal(gradeMember(null, base), "STRUCTURAL", "no base match = added member");
+ assert.equal(gradeMember(base, { signature: "f(a)", rawHash: "r1", normHash: "n1" }), "NONE");
+ assert.equal(gradeMember(base, { signature: "f(a)", rawHash: "r2", normHash: "n1" }), "FORMATTING");
+ assert.equal(gradeMember(base, { signature: "f(a)", rawHash: "r2", normHash: "n2" }), "BEHAVIORAL");
+ assert.equal(gradeMember(base, { signature: "f(a,b)", rawHash: "r2", normHash: "n2" }), "STRUCTURAL");
+});
+
+test("maxLevel and atLeast order NONE < FORMATTING < BEHAVIORAL < STRUCTURAL", () => {
+ assert.equal(maxLevel("NONE", "FORMATTING", "BEHAVIORAL"), "BEHAVIORAL");
+ assert.equal(maxLevel("NONE", "NONE"), "NONE");
+ assert.equal(maxLevel(), "NONE");
+ assert.ok(atLeast("STRUCTURAL", "BEHAVIORAL"));
+ assert.ok(atLeast("BEHAVIORAL", "BEHAVIORAL"));
+ assert.ok(!atLeast("FORMATTING", "BEHAVIORAL"));
+ assert.ok(!atLeast("NONE", "FORMATTING"));
+});
+
+test("gradeFile rolls members up, treats import change as STRUCTURAL, floors trivia at FORMATTING", () => {
+ assert.equal(gradeFile({ memberLevels: ["NONE", "BEHAVIORAL"], rawFileChanged: true }), "BEHAVIORAL");
+ assert.equal(gradeFile({ memberLevels: ["NONE"], importExportChanged: true, rawFileChanged: true }), "STRUCTURAL");
+ assert.equal(gradeFile({ memberLevels: ["NONE"], rawFileChanged: true }), "FORMATTING", "top-level trivia floor");
+ assert.equal(gradeFile({ memberLevels: ["NONE"], rawFileChanged: false }), "NONE");
+});
+
+// === End-to-end grading through the extractor ===
+
+// Parse one changed file, diffing `base` against `head`, and return the graded
+// records. `head` is what lands on disk; `base` is fed as the prior version.
+async function gradeFileChange(name, language, base, head) {
+ const dir = await mkdtemp(join(tmpdir(), "tinker-fp-"));
+ try {
+ await writeFile(join(dir, name), head);
+ const r = await extract(dir, language, {
+ changedFiles: [name],
+ baseContents: { [name]: base },
+ expandHierarchy: false,
+ });
+ return r;
+ } finally {
+ await rm(dir, { recursive: true, force: true });
+ }
+}
+
+const JAVA_BASE =
+ "package x;\npublic class Calc {\n public int add(int a, int b) {\n return a + b;\n }\n}\n";
+
+test("Java: identical content grades NONE", async () => {
+ const r = await gradeFileChange("Calc.java", "java", JAVA_BASE, JAVA_BASE);
+ assert.equal(r.functions.find((f) => f.name === "add").changeLevel, "NONE");
+ assert.equal(r.files.find((f) => f.path === "Calc.java").changeLevel, "NONE");
+});
+
+test("Java: comment/whitespace-only edit grades FORMATTING", async () => {
+ const head =
+ "package x;\npublic class Calc {\n public int add(int a, int b) {\n // sum the two operands\n return a + b;\n }\n}\n";
+ const r = await gradeFileChange("Calc.java", "java", JAVA_BASE, head);
+ assert.equal(r.functions.find((f) => f.name === "add").changeLevel, "FORMATTING");
+});
+
+test("Java: body logic change grades BEHAVIORAL", async () => {
+ const head =
+ "package x;\npublic class Calc {\n public int add(int a, int b) {\n return a - b;\n }\n}\n";
+ const r = await gradeFileChange("Calc.java", "java", JAVA_BASE, head);
+ assert.equal(r.functions.find((f) => f.name === "add").changeLevel, "BEHAVIORAL");
+});
+
+test("Java: signature change grades STRUCTURAL", async () => {
+ const head =
+ "package x;\npublic class Calc {\n public int add(int a, int b, int c) {\n return a + b + c;\n }\n}\n";
+ const r = await gradeFileChange("Calc.java", "java", JAVA_BASE, head);
+ assert.equal(r.functions.find((f) => f.name === "add").changeLevel, "STRUCTURAL");
+});
+
+test("Java: an untouched helper in a changed file grades NONE", async () => {
+ const base =
+ "package x;\npublic class Calc {\n public int add(int a, int b) {\n return a + b;\n }\n public int untouched(int a) {\n return a;\n }\n}\n";
+ const head =
+ "package x;\npublic class Calc {\n public int add(int a, int b) {\n return a - b;\n }\n public int untouched(int a) {\n return a;\n }\n}\n";
+ const r = await gradeFileChange("Calc.java", "java", base, head);
+ assert.equal(r.functions.find((f) => f.name === "add").changeLevel, "BEHAVIORAL");
+ assert.equal(r.functions.find((f) => f.name === "untouched").changeLevel, "NONE",
+ "the helper the PR did not touch is not in the changed set");
+});
+
+// Python exercises the comment-skip on a second grammar (comment node type
+// "comment") and significant indentation.
+const PY_BASE = "class Calc:\n def add(self, a, b):\n return a + b\n";
+
+test("Python: comment-only edit grades FORMATTING", async () => {
+ const head = "class Calc:\n def add(self, a, b):\n # add them\n return a + b\n";
+ const r = await gradeFileChange("calc.py", "python", PY_BASE, head);
+ assert.equal(r.functions.find((f) => f.name === "add").changeLevel, "FORMATTING");
+});
+
+test("Python: body change grades BEHAVIORAL", async () => {
+ const head = "class Calc:\n def add(self, a, b):\n return a * b\n";
+ const r = await gradeFileChange("calc.py", "python", PY_BASE, head);
+ assert.equal(r.functions.find((f) => f.name === "add").changeLevel, "BEHAVIORAL");
+});