From 73f7e7e9fb1628f61795fbf83f4c845d3c12e285 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Sun, 23 Aug 2026 19:19:00 -0300 Subject: [PATCH] feat(review-tutor): add structure analysis core and endpoint Parse the input diff without the DOM, track renames, analyze TypeScript and JavaScript connections lexically with bounded exact evidence, and serve a deterministic, cached, honestly partial structure snapshot from an authenticated local route. --- packages/review-tutor/src/protocol.ts | 69 ++ packages/review-tutor/src/server-session.ts | 14 +- packages/review-tutor/src/server.ts | 1 + packages/review-tutor/src/structure.ts | 811 +++++++++++++++++++ packages/review-tutor/test/structure.test.ts | 726 +++++++++++++++++ 5 files changed, 1620 insertions(+), 1 deletion(-) create mode 100644 packages/review-tutor/src/structure.ts create mode 100644 packages/review-tutor/test/structure.test.ts diff --git a/packages/review-tutor/src/protocol.ts b/packages/review-tutor/src/protocol.ts index bd87b9a..ee241ab 100644 --- a/packages/review-tutor/src/protocol.ts +++ b/packages/review-tutor/src/protocol.ts @@ -12,6 +12,18 @@ export const LIMITS = { stderr: 32 * 1024, } as const; +export const STRUCTURE_LIMITS = { + maxFiles: 200, + maxEdges: 2000, + maxEvidencePerEdge: 4, + evidenceText: 200, + lineLength: 4000, + diffLines: 200_000, + omittedRows: 200, + statementsPerFile: 2000, + externalNames: 8, +} as const; + export type QuizOutcome = "got_it" | "almost" | "review_again"; export type SourceRequest = @@ -93,6 +105,63 @@ export interface LearningEntry { updatedAt: string; } +export interface StructureComparison { + kind: InputSnapshot["kind"]; + label: string; + from: string; + to: string; + partial: boolean; + reasons: string[]; +} + +export interface StructureFile { + path: string; + status: "added" | "removed" | "modified" | "renamed"; + renamedFrom?: string; + additions: number; + deletions: number; + analyzed: boolean; + reason?: string; +} + +export interface StructureEvidence { + path: string; + line: number; + text: string; +} + +export interface StructureEdge { + from: string; + to: string; + kind: "import" | "reexport" | "require" | "dynamic-import"; + typeOnly: boolean; + status: "added" | "removed" | "modified" | "unchanged"; + specifier: string; + evidence: StructureEvidence[]; +} + +export interface StructureOmission { + path?: string; + reason: string; +} + +export interface StructureLimits { + maxFiles: typeof STRUCTURE_LIMITS.maxFiles; + maxEdges: typeof STRUCTURE_LIMITS.maxEdges; + maxEvidencePerEdge: typeof STRUCTURE_LIMITS.maxEvidencePerEdge; + truncated: boolean; + omitted: StructureOmission[]; +} + +export interface StructureSnapshot { + protocol: "rt/1"; + inputId: string; + comparison: StructureComparison; + files: StructureFile[]; + edges: StructureEdge[]; + limits: StructureLimits; +} + export type SseEventType = "hello" | "state" | "question" | "answer_delta" | "source" | "log_update" | "error" | "bye"; export interface SseEvent { id: number; type: SseEventType; data: unknown } diff --git a/packages/review-tutor/src/server-session.ts b/packages/review-tutor/src/server-session.ts index 7064445..d162900 100644 --- a/packages/review-tutor/src/server-session.ts +++ b/packages/review-tutor/src/server-session.ts @@ -4,9 +4,10 @@ import { exportLearningHtml } from "./export-html.ts"; import { loadInput, type ExecFile } from "./inputs.ts"; import { appendEntry, foldLog, persistInput, updateEntry } from "./log.ts"; import { buildTutorPrompt } from "./prompt.ts"; -import { LIMITS, validateAskRequest, validateLogPatch, validateSourceRequest, type AskRequest, type InputSnapshot, type LearningEntry, type ModelChoice, type QuestionView } from "./protocol.ts"; +import { LIMITS, validateAskRequest, validateLogPatch, validateSourceRequest, type AskRequest, type InputSnapshot, type LearningEntry, type ModelChoice, type QuestionView, type StructureSnapshot } from "./protocol.ts"; import type { StatePaths } from "./paths.ts"; import type { SseHub } from "./sse.ts"; +import { structureSnapshotFor } from "./structure.ts"; export interface RunnerLike { run(request: { provider: string; model: string; thinking: string; cwd: string; prompt: string }, delta: (text: string) => void): Promise<{ answer: string; usage?: Record }>; @@ -64,6 +65,7 @@ export class ReviewTutorSession { private readonly queue: string[] = []; private readonly threadHistory: LearningEntry[] = []; private currentInput?: InputSnapshot; + private structureCache?: { inputId: string; snapshot: StructureSnapshot }; private running?: string; private lastHeartbeat = 0; private closing = false; @@ -280,6 +282,16 @@ export class ReviewTutorSession { } } + structure(): SessionReply { + const input = this.currentInput; + if (!input) return { status: 409, value: { + error: "structure failed: expected a loaded input snapshot, received none; load a source and retry" } }; + if (this.structureCache?.inputId !== input.id) { + this.structureCache = { inputId: input.id, snapshot: structureSnapshotFor(input) }; + } + return { status: 200, value: this.structureCache.snapshot }; + } + async log(url: URL): Promise { const limit = parseLogLimit(url.searchParams.get("limit")); const entries = await foldLog(this.paths); diff --git a/packages/review-tutor/src/server.ts b/packages/review-tutor/src/server.ts index 4fe41f7..dc1c2f1 100644 --- a/packages/review-tutor/src/server.ts +++ b/packages/review-tutor/src/server.ts @@ -125,6 +125,7 @@ async function primaryRoute(request: IncomingMessage, response: ServerResponse, async function secondaryRoute(request: IncomingMessage, response: ServerResponse, url: URL, session: ReviewTutorSession): Promise { const method = request.method; const path = url.pathname; + if (method === "GET" && path === "/api/structure") return send(response, session.structure()); if (method === "GET" && path === "/api/log") return send(response, await session.log(url)); const log = path.match(/^\/api\/log\/([^/]+)$/); if (method === "PATCH" && log) return send(response, await session.patchLog(request, log[1]!)); diff --git a/packages/review-tutor/src/structure.ts b/packages/review-tutor/src/structure.ts new file mode 100644 index 0000000..56c3202 --- /dev/null +++ b/packages/review-tutor/src/structure.ts @@ -0,0 +1,811 @@ +import { posix } from "node:path"; +import { + STRUCTURE_LIMITS, + type InputSnapshot, + type StructureComparison, + type StructureEdge, + type StructureEvidence, + type StructureFile, + type StructureLimits, + type StructureOmission, + type StructureSnapshot, +} from "./protocol.ts"; + +type LineOrigin = "add" | "del" | "context"; +type Side = "new" | "old"; + +interface DiffLine { origin: LineOrigin; oldLine: number; newLine: number; text: string } +interface DiffHunk { lines: DiffLine[] } +interface DiffFile { + path: string; + oldPath?: string; + status: StructureFile["status"]; + additions: number; + deletions: number; + binary: boolean; + hunks: DiffHunk[]; +} +interface ParsedDiff { + files: DiffFile[]; + reasons: string[]; + extraFiles: number; + longLines: number; +} + +interface DocLine { number: number; origin: LineOrigin; text: string; start: number } +interface Doc { text: string; lines: DocLine[] } +interface Statement { + kind: StructureEdge["kind"]; + specifier: string; + typeOnly: boolean; + offset: number; + specifierOffset: number; +} + +interface ResolutionIndex { current: Map; previous: Map } + +interface Collector { + edges: Map; + omitted: StructureOmission[]; + external: Map>; + unresolved: boolean; + truncated: boolean; + suppressed: number; + droppedEdges: number; +} + +const ANALYZED_EXTENSIONS = [".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", ".mjs", ".cjs"]; +const EXTENSIONLESS_CANDIDATES = [".ts", ".tsx", ".mts", ".cts", ".js", ".jsx", ".mjs", ".cjs"]; +const JS_TO_TS: Record = { + ".js": [".ts", ".tsx"], + ".jsx": [".tsx"], + ".mjs": [".mts"], + ".cjs": [".cts"], +}; +/** Statement keywords, ignoring member access such as `foo.import`. */ +const STATEMENT_KEYWORD = /(? right ? 1 : 0; +} + +function extensionOf(path: string): string { + const name = path.slice(path.lastIndexOf("/") + 1); + const match = /\.[A-Za-z0-9]+$/.exec(name); + return match ? match[0].toLowerCase() : ""; +} + +function unquotePath(value: string): string { + if (!value.startsWith("\"") || !value.endsWith("\"") || value.length < 2) return value; + return value.slice(1, -1).replace(/\\(["\\])/g, "$1"); +} + +function stripSidePrefix(value: string): string { + const path = unquotePath(value.trim()); + return path.replace(/^[ab]\//, ""); +} + +function splitGitHeader(line: string): { old: string; next: string } | undefined { + const rest = line.slice("diff --git ".length); + const same = /^"?a\/(.+?)"? "?b\/\1"?$/.exec(rest); + if (same) return { old: unquotePath(same[1]!), next: unquotePath(same[1]!) }; + const split = rest.lastIndexOf(" b/"); + if (split < 0) return undefined; + return { old: stripSidePrefix(rest.slice(0, split)), next: stripSidePrefix(rest.slice(split + 1)) }; +} + +function startFile(line: string): DiffFile { + const header = splitGitHeader(line); + return { path: header?.next ?? "", status: "modified", additions: 0, deletions: 0, binary: false, hunks: [] }; +} + +function applyFileHeader(file: DiffFile, line: string): void { + if (line.startsWith("new file mode")) file.status = "added"; + else if (line.startsWith("deleted file mode")) file.status = "removed"; + else if (line.startsWith("rename from ")) { file.oldPath = stripSidePrefix(line.slice(12)); file.status = "renamed"; } + else if (line.startsWith("rename to ")) file.path = stripSidePrefix(line.slice(10)); + else if (line.startsWith("copy to ")) { file.path = stripSidePrefix(line.slice(8)); file.status = "added"; } + else if (line.startsWith("Binary files ") || line.startsWith("GIT binary patch")) file.binary = true; + else if (line.startsWith("--- ")) { + const value = line.slice(4).trim(); + if (value === "/dev/null") file.status = "added"; + else if (file.status !== "renamed") file.oldPath = stripSidePrefix(value); + } else if (line.startsWith("+++ ")) { + const value = line.slice(4).trim(); + if (value === "/dev/null") file.status = "removed"; + else if (file.status !== "renamed") file.path = stripSidePrefix(value); + } +} + +function displayPath(file: DiffFile): string { + return file.path || file.oldPath || ""; +} + +interface ParseState { + file?: DiffFile; + hunk?: DiffHunk; + oldLine: number; + newLine: number; + lineNumber: number; + files: DiffFile[]; + reasons: string[]; + extraFiles: number; + longLines: number; + merge: boolean; +} + +function originOf(line: string): LineOrigin | undefined { + if (line.startsWith("+")) return "add"; + if (line.startsWith("-")) return "del"; + if (line.startsWith(" ") || line === "") return "context"; + return undefined; +} + +function appendHunkLine(state: ParseState, file: DiffFile, hunk: DiffHunk, line: string): void { + const origin = originOf(line); + if (!origin) { + state.hunk = undefined; + applyFileHeader(file, line); + return; + } + hunk.lines.push({ origin, oldLine: state.oldLine, newLine: state.newLine, text: line.slice(1) }); + if (origin !== "add") state.oldLine += 1; + if (origin !== "del") state.newLine += 1; + if (origin === "add") file.additions += 1; + if (origin === "del") file.deletions += 1; +} + +function startDiffFile(state: ParseState, line: string): void { + state.hunk = undefined; + if (state.files.length >= STRUCTURE_LIMITS.maxFiles) { + state.extraFiles += 1; + state.file = undefined; + return; + } + state.file = startFile(line); + state.files.push(state.file); +} + +function consumeDiffLine(state: ParseState, line: string): void { + if (line.startsWith("diff --cc ") || line.startsWith("diff --combined ") || line.startsWith("@@@")) { + state.merge = true; + state.file = undefined; + state.hunk = undefined; + return; + } + if (line.startsWith("diff --git ")) return startDiffFile(state, line); + const file = state.file; + if (!file) return; + const range = /^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@/.exec(line); + if (range) { + state.oldLine = Number(range[1]); + state.newLine = Number(range[2]); + state.hunk = { lines: [] }; + file.hunks.push(state.hunk); + return; + } + if (line.startsWith("@@")) { + state.hunk = undefined; + state.reasons.push(`malformed hunk header at diff line ${state.lineNumber}: no import data for that file`); + return; + } + if (!state.hunk) return applyFileHeader(file, line); + if (line.startsWith("\\")) return; + appendHunkLine(state, file, state.hunk, line); +} + +/** Reads at most `diffLines` lines without materializing the whole content as an array. */ +function scanDiffLines(content: string, state: ParseState): boolean { + let start = 0; + while (state.lineNumber < STRUCTURE_LIMITS.diffLines) { + const end = content.indexOf("\n", start); + const raw = end < 0 ? content.slice(start) : content.slice(start, end); + state.lineNumber += 1; + const capped = raw.length > STRUCTURE_LIMITS.lineLength; + if (capped) state.longLines += 1; + consumeDiffLine(state, capped ? raw.slice(0, STRUCTURE_LIMITS.lineLength) : raw); + if (end < 0) return true; + start = end + 1; + } + return false; +} + +export function parseUnifiedDiff(content: string): ParsedDiff { + const state: ParseState = { + oldLine: 0, newLine: 0, lineNumber: 0, files: [], reasons: [], extraFiles: 0, longLines: 0, merge: false, + }; + const complete = scanDiffLines(content, state); + if (!complete) { + state.reasons.push(`diff parsing stopped after ${STRUCTURE_LIMITS.diffLines} lines: no import data for the rest of this comparison`); + } + const files = state.files.filter((candidate) => displayPath(candidate)); + if (state.merge) { + state.reasons.push("merge commit: combined diffs are not analyzed; pick one parent range (e.g. ^1...) and retry"); + } else if (!files.length && content.trim()) { + state.reasons.push("diff parse failed: expected unified diff file headers such as 'diff --git', received none; load a Git-based source and retry"); + } + return { files, reasons: state.reasons, extraFiles: state.extraFiles, longLines: state.longLines }; +} + +function maskString(text: string, out: string[], start: number): number { + const quote = text[start]!; + let index = start + 1; + while (index < text.length) { + const character = text[index]!; + if (character === "\\") { + out[index] = " "; + if (text[index + 1] !== undefined && text[index + 1] !== "\n") out[index + 1] = " "; + index += 2; + continue; + } + if (character === quote) return index + 1; + if (character === "\n" && quote !== "`") return index; + if (character !== "\n") out[index] = " "; + index += 1; + } + return index; +} + +function maskLineComment(text: string, out: string[], start: number): number { + let index = start; + while (index < text.length && text[index] !== "\n") { out[index] = " "; index += 1; } + return index; +} + +function maskBlockComment(text: string, out: string[], start: number): number { + out[start] = " "; + out[start + 1] = " "; + let index = start + 2; + while (index < text.length && !(text[index] === "*" && text[index + 1] === "/")) { + if (text[index] !== "\n") out[index] = " "; + index += 1; + } + if (index < text.length) { out[index] = " "; out[index + 1] = " "; } + return index + 2; +} + +function skipBackWhitespace(text: ArrayLike, from: number): number { + let cursor = from; + while (cursor >= 0 && (text[cursor] === " " || text[cursor] === "\t" || text[cursor] === "\n")) cursor -= 1; + return cursor; +} + +/** The identifier ending at `cursor`, or "" when it is longer than an operator keyword. */ +function wordBefore(text: string[], cursor: number): string { + let start = cursor; + while (start >= 0 && WORD_CHARACTER.test(text[start]!)) { + if (cursor - start > 12) return ""; + start -= 1; + } + return text.slice(start + 1, cursor + 1).join(""); +} + +/** True when a `/` at `start` opens a regex literal rather than a division. */ +function opensRegex(out: string[], start: number): boolean { + const cursor = skipBackWhitespace(out, start - 1); + if (cursor < 0) return true; + const previous = out[cursor]!; + if (REGEX_START.has(previous)) return true; + if (previous === ">" && cursor > 0 && out[cursor - 1] === "=") return true; + if (!WORD_CHARACTER.test(previous)) return false; + const word = wordBefore(out, cursor); + if (!EXPRESSION_KEYWORDS.has(word)) return false; + const beforeWord = skipBackWhitespace(out, cursor - word.length); + return beforeWord < 0 || out[beforeWord] !== "."; +} + +function regexEnd(text: string, start: number): number | undefined { + let index = start + 1; + while (index < text.length && text[index] !== "\n") { + if (text[index] === "\\") { index += 2; continue; } + if (text[index] === "/") return index; + index += 1; + } + return undefined; +} + +function maskRegexLiteral(text: string, out: string[], start: number): number | undefined { + if (!opensRegex(out, start)) return undefined; + const end = regexEnd(text, start); + if (end === undefined) return undefined; + for (let index = start; index <= end; index += 1) out[index] = " "; + return end + 1; +} + +/** + * Blanks comment, string, and regex-literal bodies while preserving every UTF-16 + * offset, so no keyword inside them can be read as an import statement. + */ +export function maskLiterals(text: string): string { + const out = text.split(""); + let index = 0; + while (index < text.length) { + const character = text[index]!; + const next = text[index + 1]; + if (character === "/" && next === "/") index = maskLineComment(text, out, index); + else if (character === "/" && next === "*") index = maskBlockComment(text, out, index); + else if (character === "'" || character === "\"" || character === "`") index = maskString(text, out, index); + else if (character === "/") index = maskRegexLiteral(text, out, index) ?? index + 1; + else index += 1; + } + return out.join(""); +} + +function buildDoc(file: DiffFile, side: Side): Doc { + const lines: DocLine[] = []; + const parts: string[] = []; + let offset = 0; + const push = (number: number, origin: LineOrigin, text: string): void => { + lines.push({ number, origin, text, start: offset }); + parts.push(text); + offset += text.length + 1; + }; + file.hunks.forEach((hunk, index) => { + if (index > 0) push(0, "context", ";"); + for (const line of hunk.lines) { + if (side === "new" && line.origin === "del") continue; + if (side === "old" && line.origin === "add") continue; + push(side === "new" ? line.newLine : line.oldLine, line.origin, line.text); + } + }); + return { text: parts.join("\n"), lines }; +} + +function lineAt(doc: Doc, offset: number): DocLine | undefined { + let low = 0; + let high = doc.lines.length - 1; + let found: DocLine | undefined; + while (low <= high) { + const middle = (low + high) >> 1; + const line = doc.lines[middle]!; + if (line.start <= offset) { found = line; low = middle + 1; } else high = middle - 1; + } + return found && found.number !== 0 ? found : undefined; +} + +/** + * The specifier line decides a connection's state: a multi-line statement whose + * module string changed is an added edge on the new side and a removed edge on the + * old side. Unchanged context lines are reported once, from the post-change side. + */ +function specifierLineFor(doc: Doc, offset: number, side: Side): DocLine | undefined { + const line = lineAt(doc, offset); + if (!line) return undefined; + if (side === "old" && line.origin !== "del") return undefined; + return line; +} + +function pushFromStatement(doc: Doc, mask: string, at: number, statements: Statement[]): boolean { + FROM_TAIL.lastIndex = at; + const match = FROM_TAIL.exec(mask); + if (!match || NESTED_KEYWORD.test(match[2]!)) return false; + const whole = match[0]; + const open = whole.lastIndexOf(match[3]!, whole.length - 2); + if (open < 0) return false; + statements.push({ + kind: match[1] === "export" ? "reexport" : "import", + specifier: doc.text.slice(at + open + 1, at + whole.length - 1), + typeOnly: /^\s*type\b/.test(match[2]!), + offset: at, + specifierOffset: at + open, + }); + return true; +} + +function pushBareImport(doc: Doc, mask: string, at: number, statements: Statement[]): void { + BARE_TAIL.lastIndex = at; + const match = BARE_TAIL.exec(mask); + if (!match) return; + const whole = match[0]; + const open = whole.indexOf(match[1]!); + statements.push({ + kind: "import", + specifier: doc.text.slice(at + open + 1, at + whole.length - 1), + typeOnly: false, + offset: at, + specifierOffset: at + open, + }); +} + +/** + * True when an import clause runs past the work bound before its module string, + * which is a statement the scanner must report rather than silently drop. + */ +function overlongClause(mask: string, at: number, keyword: string): boolean { + let index = at + keyword.length; + const stop = Math.min(mask.length, index + CLAUSE_SCAN); + while (index < stop && CLAUSE_CHARACTER.test(mask[index]!)) index += 1; + if (index - (at + keyword.length) <= CLAUSE_LIMIT) return false; + return mask[index] === "'" || mask[index] === "\""; +} + +function scanStatementKeywords(doc: Doc, mask: string, statements: Statement[], overlong: number[]): void { + for (const keyword of mask.matchAll(STATEMENT_KEYWORD)) { + if (pushFromStatement(doc, mask, keyword.index, statements)) continue; + if (overlongClause(mask, keyword.index, keyword[1]!)) { + overlong.push(keyword.index); + continue; + } + pushBareImport(doc, mask, keyword.index, statements); + } +} + +/** `shim . require("x")` and `shim ?. import("x")` are member calls, not module loads. */ +function memberCall(mask: string, at: number): boolean { + const cursor = skipBackWhitespace(mask, at - 1); + return cursor >= 0 && mask[cursor] === "."; +} + +function scanCallStatements(doc: Doc, mask: string, statements: Statement[], nonLiteral: number[]): void { + for (const match of mask.matchAll(CALL_IMPORT)) { + if (memberCall(mask, match.index)) continue; + const start = match.index + match[0].length; + const quote = mask[start]; + const close = quote === "'" || quote === "\"" ? mask.indexOf(quote, start + 1) : -1; + if (close < 0 || !/^\s*\)/.test(mask.slice(close + 1, close + 8))) { + nonLiteral.push(match.index); + continue; + } + statements.push({ + kind: match[1] === "require" ? "require" : "dynamic-import", + specifier: doc.text.slice(start + 1, close), + typeOnly: false, + offset: match.index, + specifierOffset: start, + }); + } +} + +/** Lexical, non-executing scan of one reconstructed document side. */ +function scanDoc(doc: Doc): { statements: Statement[]; nonLiteral: number[]; overlong: number[] } { + const mask = maskLiterals(doc.text); + const statements: Statement[] = []; + const nonLiteral: number[] = []; + const overlong: number[] = []; + scanStatementKeywords(doc, mask, statements, overlong); + scanCallStatements(doc, mask, statements, nonLiteral); + return { statements, nonLiteral, overlong }; +} + +function candidatePaths(base: string): string[] { + const list = [base]; + const extension = extensionOf(base); + for (const replacement of JS_TO_TS[extension] ?? []) { + list.push(`${base.slice(0, -extension.length)}${replacement}`); + } + if (!extension) { + for (const candidate of EXTENSIONLESS_CANDIDATES) list.push(`${base}${candidate}`); + for (const candidate of EXTENSIONLESS_CANDIDATES) list.push(`${base}/index${candidate}`); + } + return list; +} + +/** + * Candidate-path matching against the changed-file set only; no Node resolution, + * no filesystem access. Old-side lookups consult pre-rename paths first because the + * pre-change tree still held them; new-side lookups never do. + */ +function resolveSpecifier(from: string, specifier: string, index: ResolutionIndex, side: Side): string | undefined { + const base = posix.normalize(posix.join(posix.dirname(from), specifier.replace(/\\/g, "/"))).replace(/\/+$/, ""); + for (const candidate of candidatePaths(base)) { + const resolved = side === "old" ? index.previous.get(candidate) ?? index.current.get(candidate) : index.current.get(candidate); + if (resolved) return resolved; + } + return undefined; +} + +/** Deterministic UTF-16 trimming; control characters other than tab become U+FFFD. */ +function trimText(text: string): string { + const clean = text.trim().replace(CONTROL_CHARACTERS, "\uFFFD"); + return clean.length > STRUCTURE_LIMITS.evidenceText ? clean.slice(0, STRUCTURE_LIMITS.evidenceText) : clean; +} + +function omit(collector: Collector, omission: StructureOmission): void { + if (collector.omitted.some((existing) => existing.path === omission.path && existing.reason === omission.reason)) return; + if (collector.omitted.length >= STRUCTURE_LIMITS.omittedRows) { + collector.suppressed += 1; + return; + } + collector.omitted.push(omission); +} + +function addEvidence(edge: StructureEdge, evidence: StructureEvidence[]): void { + for (const item of evidence) { + if (edge.evidence.length >= STRUCTURE_LIMITS.maxEvidencePerEdge) return; + if (edge.evidence.some((existing) => existing.path === item.path && existing.line === item.line)) continue; + edge.evidence.push(item); + } +} + +function addEdge(collector: Collector, edge: StructureEdge, evidence: StructureEvidence[]): void { + const key = [edge.from, edge.to, edge.kind, String(edge.typeOnly), edge.status].join("\u0000"); + const existing = collector.edges.get(key); + if (existing) return addEvidence(existing, evidence); + if (collector.edges.size >= STRUCTURE_LIMITS.maxEdges) { + collector.droppedEdges += 1; + collector.truncated = true; + return; + } + const created: StructureEdge = { ...edge, evidence: [] }; + collector.edges.set(key, created); + addEvidence(created, evidence); +} + +function evidenceFor(file: StructureFile, doc: Doc, statement: Statement, specifier: DocLine): StructureEvidence[] { + const first = lineAt(doc, statement.offset); + const lines = first && first.number !== specifier.number ? [first, specifier] : [specifier]; + return lines.map((line) => ({ path: file.path, line: line.number, text: trimText(line.text) })); +} + +function recordStatement( + collector: Collector, file: StructureFile, doc: Doc, side: Side, + statement: Statement, index: ResolutionIndex, +): void { + const specifier = specifierLineFor(doc, statement.specifierOffset, side); + if (!specifier) return; + if (!statement.specifier.startsWith(".")) { + const bucket = collector.external.get(file.path) ?? new Set(); + bucket.add(statement.specifier); + collector.external.set(file.path, bucket); + return; + } + const base = side === "old" ? file.renamedFrom ?? file.path : file.path; + const target = resolveSpecifier(base, statement.specifier, index, side); + if (!target) { + collector.unresolved = true; + omit(collector, { path: file.path, reason: trimText( + `specifier '${statement.specifier}' at line ${specifier.number} matches no changed file: no import data outside the changed set`) }); + return; + } + const status = specifier.origin === "add" ? "added" : specifier.origin === "del" ? "removed" : "unchanged"; + addEdge(collector, { + from: file.path, to: target, kind: statement.kind, typeOnly: statement.typeOnly, + status, specifier: statement.specifier, evidence: [], + }, evidenceFor(file, doc, statement, specifier)); +} + +function analyzeSide( + collector: Collector, file: StructureFile, diff: DiffFile, side: Side, + index: ResolutionIndex, budget: { left: number }, +): void { + const doc = buildDoc(diff, side); + if (!doc.text) return; + const { statements, nonLiteral, overlong } = scanDoc(doc); + const allowed = statements.slice(0, Math.max(0, budget.left)); + budget.left -= statements.length; + if (budget.left < 0) { + collector.truncated = true; + omit(collector, { path: file.path, reason: + `statement cap reached: expected at most ${STRUCTURE_LIMITS.statementsPerFile} import statements in this file, received more; later statements are not analyzed` }); + } + for (const statement of allowed) recordStatement(collector, file, doc, side, statement, index); + for (const offset of nonLiteral) { + const line = specifierLineFor(doc, offset, side); + if (!line) continue; + collector.unresolved = true; + omit(collector, { path: file.path, reason: `non-literal specifier at line ${line.number}: no import data for that call` }); + } + for (const offset of overlong) { + const line = specifierLineFor(doc, offset, side); + if (!line) continue; + collector.truncated = true; + omit(collector, { path: file.path, reason: + `import clause longer than ${CLAUSE_LIMIT} characters at line ${line.number}: no import data for that statement` }); + } +} + +function describeFile(diff: DiffFile): StructureFile { + const path = displayPath(diff); + const extension = extensionOf(path); + const base: StructureFile = { + path, + status: diff.status, + ...(diff.status === "renamed" && diff.oldPath ? { renamedFrom: diff.oldPath } : {}), + additions: diff.additions, + deletions: diff.deletions, + analyzed: false, + }; + if (diff.binary) return { ...base, reason: "binary content: no import data" }; + if (!ANALYZED_EXTENSIONS.includes(extension)) { + return { ...base, reason: `unsupported file type '${extension || "none"}': no import data` }; + } + if (!diff.hunks.length) return { ...base, reason: "no diff content for this file: no import data" }; + return { ...base, analyzed: true }; +} + +function mergeEvidence(removed: StructureEvidence[], added: StructureEvidence[]): StructureEvidence[] { + const cap = STRUCTURE_LIMITS.maxEvidencePerEdge; + const fromRemoved = Math.min(removed.length, Math.max(Math.floor(cap / 2), cap - added.length)); + return [...removed.slice(0, fromRemoved), ...added.slice(0, cap - fromRemoved)]; +} + +/** An import whose bindings changed is one modified connection, not an add plus a remove. */ +function collapseModified(edges: StructureEdge[]): StructureEdge[] { + const groups = new Map(); + for (const edge of edges) { + const key = [edge.from, edge.to, edge.kind, String(edge.typeOnly)].join("\u0000"); + groups.set(key, [...groups.get(key) ?? [], edge]); + } + const result: StructureEdge[] = []; + for (const group of groups.values()) { + const added = group.find((edge) => edge.status === "added"); + const removed = group.find((edge) => edge.status === "removed"); + if (!added || !removed) { + result.push(...group); + continue; + } + result.push(...group.filter((edge) => edge !== added && edge !== removed)); + result.push({ ...added, status: "modified", evidence: mergeEvidence(removed.evidence, added.evidence) }); + } + return result; +} + +function sortEdges(edges: StructureEdge[]): StructureEdge[] { + return edges.sort((left, right) => + compareText(left.from, right.from) || compareText(left.to, right.to) + || compareText(left.kind, right.kind) + || (left.evidence[0]?.line ?? 0) - (right.evidence[0]?.line ?? 0) + || compareText(left.specifier, right.specifier) + || Number(left.typeOnly) - Number(right.typeOnly) + || compareText(left.status, right.status)); +} + +function comparisonEndpoints(input: InputSnapshot): { from: string; to: string } { + if (input.kind === "worktree") return { from: "index", to: "working tree" }; + if (input.kind === "staged") return { from: "HEAD", to: "index" }; + if (input.kind === "commit") return { from: "first parent", to: `commit ${input.label.replace(/^Commit /, "")}` }; + if (input.kind === "range") { + const separator = input.label.indexOf("..."); + return { from: "merge-base", to: separator < 0 ? input.label : input.label.slice(separator + 3) }; + } + if (input.kind === "pr") return { from: "base", to: `head ${input.headSha ?? "unknown"}` }; + return { from: "unavailable", to: "unavailable" }; +} + +function comparisonOf(input: InputSnapshot, reasons: string[]): StructureComparison { + const endpoints = comparisonEndpoints(input); + return { kind: input.kind, label: input.label, from: endpoints.from, to: endpoints.to, partial: reasons.length > 0, reasons }; +} + +function omissionRank(omission: StructureOmission): number { + if (omission.reason.startsWith("further omissions not listed")) return 2; + return omission.path === undefined ? 1 : 0; +} + +function limitsOf(truncated: boolean, omitted: StructureOmission[]): StructureLimits { + return { + maxFiles: STRUCTURE_LIMITS.maxFiles, + maxEdges: STRUCTURE_LIMITS.maxEdges, + maxEvidencePerEdge: STRUCTURE_LIMITS.maxEvidencePerEdge, + truncated, + omitted: omitted.sort((left, right) => + omissionRank(left) - omissionRank(right) + || compareText(left.path ?? "", right.path ?? "") + || compareText(left.reason, right.reason)), + }; +} + +function emptySnapshot(input: InputSnapshot, reasons: string[]): StructureSnapshot { + return { + protocol: "rt/1", + inputId: input.id, + comparison: comparisonOf(input, reasons), + files: [], + edges: [], + limits: limitsOf(false, []), + }; +} + +function collectExternal(collector: Collector): void { + for (const path of [...collector.external.keys()].sort(compareText)) { + const names = [...collector.external.get(path)!].sort(compareText); + const shown = names.slice(0, STRUCTURE_LIMITS.externalNames); + const suffix = shown.length < names.length ? ", …" : ""; + omit(collector, { path, reason: trimText(`external modules (${names.length}): ${shown.join(", ")}${suffix}`) }); + } +} + +function analyzeFiles(files: StructureFile[], diffs: Map): Collector { + const index: ResolutionIndex = { current: new Map(), previous: new Map() }; + for (const file of files) { + index.current.set(file.path, file.path); + if (file.renamedFrom) index.previous.set(file.renamedFrom, file.path); + } + const collector: Collector = { + edges: new Map(), omitted: [], external: new Map(), + unresolved: false, truncated: false, suppressed: 0, droppedEdges: 0, + }; + for (const file of files) { + if (!file.analyzed) continue; + const diff = diffs.get(file.path)!; + const budget = { left: STRUCTURE_LIMITS.statementsPerFile }; + analyzeSide(collector, file, diff, "new", index, budget); + analyzeSide(collector, file, diff, "old", index, budget); + } + collectExternal(collector); + return collector; +} + +function sourceReasons(kind: InputSnapshot["kind"]): string[] { + if (kind === "paste") { + return ["pasted code: structure analysis is unavailable because the comparison has no Git provenance; load a worktree, staged, commit, range, or pull-request source"]; + } + if (kind === "pr") return ["patch-only: base and head objects are not read locally"]; + return []; +} + +function capOmissions(collector: Collector, parsed: ParsedDiff, kept: number): void { + if (parsed.extraFiles) { + collector.truncated = true; + collector.omitted.push({ reason: `file cap reached: expected at most ${STRUCTURE_LIMITS.maxFiles} changed files, received ${kept + parsed.extraFiles}; ${parsed.extraFiles} files are omitted` }); + } + if (parsed.longLines) { + collector.truncated = true; + collector.omitted.push({ reason: `line length cap: ${parsed.longLines} lines longer than ${STRUCTURE_LIMITS.lineLength} characters were truncated; imports past that point are not analyzed` }); + } + if (collector.droppedEdges) { + collector.omitted.push({ reason: `connection cap reached: expected at most ${STRUCTURE_LIMITS.maxEdges} connections, received ${STRUCTURE_LIMITS.maxEdges + collector.droppedEdges}; ${collector.droppedEdges} connections are omitted` }); + } + if (collector.suppressed) { + collector.omitted.push({ reason: `further omissions not listed (${collector.suppressed} more)` }); + } +} + +/** + * Builds the deterministic Structure snapshot for one immutable input snapshot. + * Pure and lexical: no file reads, no child processes, no Git access, no execution. + */ +export function buildStructureSnapshot(input: InputSnapshot): StructureSnapshot { + const reasons = sourceReasons(input.kind); + if (input.kind === "paste") return emptySnapshot(input, reasons); + const parsed = parseUnifiedDiff(input.content); + reasons.push(...parsed.reasons); + const files = parsed.files.map(describeFile).sort((left, right) => compareText(left.path, right.path)); + const collector = analyzeFiles(files, new Map(parsed.files.map((file) => [displayPath(file), file] as const))); + for (const edge of collector.edges.values()) { + edge.evidence.sort((left, right) => left.line - right.line || compareText(left.path, right.path)); + } + const edges = sortEdges(collapseModified([...collector.edges.values()])); + capOmissions(collector, parsed, files.length); + if (collector.truncated) reasons.push("bounded analysis: some changed files, statements, or connections exceed the resource caps and are omitted"); + if (collector.unresolved) reasons.push("some specifiers match no changed file: no import data for those statements"); + return { + protocol: "rt/1", + inputId: input.id, + comparison: comparisonOf(input, reasons), + files, + edges, + limits: limitsOf(collector.truncated, collector.omitted), + }; +} + +/** Route-safe wrapper: a malformed input degrades to a partial snapshot instead of throwing. */ +export function structureSnapshotFor(input: InputSnapshot): StructureSnapshot { + try { + return buildStructureSnapshot(input); + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + return emptySnapshot(input, [ + `structure analysis failed: expected a parsable unified diff, received content this analyzer could not process (${message}); reload the source or switch to the Diff view`, + ]); + } +} diff --git a/packages/review-tutor/test/structure.test.ts b/packages/review-tutor/test/structure.test.ts new file mode 100644 index 0000000..ac449ed --- /dev/null +++ b/packages/review-tutor/test/structure.test.ts @@ -0,0 +1,726 @@ +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { fileURLToPath } from "node:url"; +import { afterEach, describe, expect, it } from "vitest"; +import type { InputSnapshot, StructureEdge, StructureSnapshot } from "../src/protocol.ts"; +import { startReviewTutorServer } from "../src/server.ts"; +import { buildStructureSnapshot, maskLiterals, parseUnifiedDiff, structureSnapshotFor } from "../src/structure.ts"; + +const skillPath = fileURLToPath(new URL("../skills/review-tutor/SKILL.md", import.meta.url)); +const temporaryRoots: string[] = []; + +afterEach(async () => { + while (temporaryRoots.length) { + await rm(temporaryRoots.pop()!, { recursive: true, force: true }); + } +}); + +function input(content: string, kind: InputSnapshot["kind"] = "worktree"): InputSnapshot { + return { + id: "input-1", + kind, + label: kind === "pr" ? "#7 Example by dev" : "Local worktree diff", + digest: "digest", + byteCount: Buffer.byteLength(content), + content, + }; +} + +function edgesFrom(snapshot: StructureSnapshot, from: string): StructureEdge[] { + return snapshot.edges.filter((edge) => edge.from === from); +} + +function edge(snapshot: StructureSnapshot, from: string, to: string): StructureEdge | undefined { + return snapshot.edges.find((candidate) => candidate.from === from && candidate.to === to); +} + +const RENAME_DIFF = `diff --git a/src/helper.ts b/src/helper.ts +index 1111111..2222222 100644 +--- a/src/helper.ts ++++ b/src/helper.ts +@@ -1,2 +1,2 @@ +-export const helper = (value: number) => value; ++export const helper = (value: number): number => value; + +diff --git a/src/config/index.ts b/src/config/index.ts +new file mode 100644 +index 0000000..3333333 +--- /dev/null ++++ b/src/config/index.ts +@@ -0,0 +1,2 @@ ++export interface Config { name: string } ++export const defaults: Config = { name: "review" }; +diff --git a/src/mixed.ts b/src/mixed.ts +index 4444444..5555555 100644 +--- a/src/mixed.ts ++++ b/src/mixed.ts +@@ -1,2 +1,2 @@ +-export type Mixed = string; ++export type Mixed = string | number; + export const value = 1; +diff --git a/src/old-name.ts b/src/renamed.ts +similarity index 88% +rename from src/old-name.ts +rename to src/renamed.ts +index 6666666..7777777 100644 +--- a/src/old-name.ts ++++ b/src/renamed.ts +@@ -1,4 +1,5 @@ + import { helper } from "./helper.js"; ++import type { Config } from "./config"; + import { type Mixed, value } from "./mixed.ts"; + + export const total = helper(value); +`; + +describe("diff parsing", () => { + it("tracks renames, statuses, and line counts", () => { + const parsed = parseUnifiedDiff(RENAME_DIFF); + expect(parsed.reasons).toEqual([]); + expect(parsed.files.map((file) => `${file.path}:${file.status}`)).toEqual([ + "src/helper.ts:modified", + "src/config/index.ts:added", + "src/mixed.ts:modified", + "src/renamed.ts:renamed", + ]); + const renamed = parsed.files.at(-1)!; + expect(renamed.oldPath).toBe("src/old-name.ts"); + expect(renamed.additions).toBe(1); + expect(parsed.files[0]!.additions).toBe(1); + expect(parsed.files[0]!.deletions).toBe(1); + }); + + it("reports a malformed diff as a partial snapshot instead of throwing", () => { + const snapshot = buildStructureSnapshot(input("this is not a diff at all\njust prose\n")); + expect(snapshot.files).toEqual([]); + expect(snapshot.comparison.partial).toBe(true); + expect(snapshot.comparison.reasons[0]).toContain("diff parse failed: expected unified diff file headers"); + }); +}); + +describe("connection analysis", () => { + const snapshot = buildStructureSnapshot(input(RENAME_DIFF)); + + it("keeps rename provenance on the file entry", () => { + const renamed = snapshot.files.find((file) => file.path === "src/renamed.ts")!; + expect(renamed.status).toBe("renamed"); + expect(renamed.renamedFrom).toBe("src/old-name.ts"); + expect(renamed.analyzed).toBe(true); + }); + + it("resolves a .js specifier to the .ts file under bundler conventions", () => { + const found = edge(snapshot, "src/renamed.ts", "src/helper.ts")!; + expect(found.specifier).toBe("./helper.js"); + expect(found.kind).toBe("import"); + expect(found.status).toBe("unchanged"); + expect(found.evidence).toEqual([ + { path: "src/renamed.ts", line: 1, text: "import { helper } from \"./helper.js\";" }, + ]); + }); + + it("resolves an extensionless specifier through a directory index and marks type-only imports", () => { + const found = edge(snapshot, "src/renamed.ts", "src/config/index.ts")!; + expect(found.typeOnly).toBe(true); + expect(found.status).toBe("added"); + expect(found.evidence[0]!.line).toBe(2); + }); + + it("treats an inline type specifier inside a value import as a value edge", () => { + const found = edge(snapshot, "src/renamed.ts", "src/mixed.ts")!; + expect(found.typeOnly).toBe(false); + expect(found.status).toBe("unchanged"); + }); + + it("stays complete for a Git worktree comparison", () => { + expect(snapshot.comparison).toMatchObject({ kind: "worktree", partial: false, reasons: [] }); + expect(snapshot.limits.truncated).toBe(false); + }); +}); + +const KINDS_DIFF = `diff --git a/src/target.ts b/src/target.ts +--- a/src/target.ts ++++ b/src/target.ts +@@ -1 +1,2 @@ + export const target = 1; ++export const extra = 2; +diff --git a/src/legacy.js b/src/legacy.js +--- a/src/legacy.js ++++ b/src/legacy.js +@@ -1 +1,2 @@ + module.exports = { legacy: true }; ++module.exports.more = true; +diff --git a/src/entry.ts b/src/entry.ts +--- a/src/entry.ts ++++ b/src/entry.ts +@@ -1,10 +1,16 @@ + import "./target.ts"; ++import { target } from "./target.ts"; + export { target as reexported } from "./target.ts"; ++export type { Target } from "./target.ts"; + const legacy = require("./legacy.js"); +-const gone = require("./target.ts"); + const lazy = () => import("./target.ts"); + const dynamic = (name) => import(name); + const built = require(\`./\${name}.js\`); + import { readFile } from "node:fs/promises"; + import react from "react"; + import { missing } from "./not-in-diff.ts"; +-// import { commented } from "./target.ts"; ++const sample = "import { fake } from './target.ts'"; +`; + +describe("edge kinds and honest omissions", () => { + const snapshot = buildStructureSnapshot(input(KINDS_DIFF)); + + it("supports import, re-export, require, and dynamic-import edges", () => { + const kinds = edgesFrom(snapshot, "src/entry.ts") + .map((found) => `${found.kind}:${found.to}:${found.typeOnly}:${found.status}`); + expect(kinds).toEqual([ + "require:src/legacy.js:false:unchanged", + "dynamic-import:src/target.ts:false:unchanged", + "import:src/target.ts:false:unchanged", + "import:src/target.ts:false:added", + "reexport:src/target.ts:false:unchanged", + "reexport:src/target.ts:true:added", + "require:src/target.ts:false:removed", + ]); + }); + + it("keeps side-effect imports and merges duplicate statements per state", () => { + const sideEffect = edgesFrom(snapshot, "src/entry.ts") + .find((found) => found.kind === "import" && found.status === "unchanged")!; + expect(sideEffect.specifier).toBe("./target.ts"); + expect(sideEffect.evidence).toHaveLength(1); + }); + + it("omits non-literal specifiers, external modules, and out-of-scope paths without inventing edges", () => { + const reasons = snapshot.limits.omitted.map((omission) => `${omission.path ?? ""}|${omission.reason}`); + expect(reasons).toContain("src/entry.ts|external modules (2): node:fs/promises, react"); + expect(reasons.filter((reason) => reason.includes("non-literal specifier"))).toHaveLength(2); + expect(reasons).toContain("src/entry.ts|specifier './not-in-diff.ts' at line 11 matches no changed file: no import data outside the changed set"); + expect(snapshot.comparison.partial).toBe(true); + expect(snapshot.comparison.reasons.some((reason) => reason.includes("no import data"))).toBe(true); + }); + + it("never reads an import out of a comment or a string literal", () => { + expect(edgesFrom(snapshot, "src/entry.ts").some((found) => found.specifier === "./commented.ts")).toBe(false); + expect(edgesFrom(snapshot, "src/entry.ts").some((found) => found.specifier === "./fake.ts")).toBe(false); + }); + + it("masks comments and string bodies while preserving offsets", () => { + const source = "import a from \"./x\"; // import b from \"./y\"\n/* import c from \"./z\" */"; + const masked = maskLiterals(source); + expect(masked).toHaveLength(source.length); + expect(masked).toContain("import a from \" \";"); + expect(masked.slice(20)).toMatch(/^[ \n]+$/); + }); +}); + +const REMOVAL_DIFF = `diff --git a/docs/guide.md b/docs/guide.md +--- a/docs/guide.md ++++ b/docs/guide.md +@@ -1 +1,2 @@ + # Guide ++import { fake } from "./nope.ts"; +diff --git a/assets/logo.png b/assets/logo.png +new file mode 100644 +index 0000000..8888888 +Binary files /dev/null and b/assets/logo.png differ +diff --git a/src/dropped.ts b/src/dropped.ts +deleted file mode 100644 +--- a/src/dropped.ts ++++ /dev/null +@@ -1,2 +0,0 @@ +-import { keep } from "./keep.ts"; +-export const dropped = keep; +diff --git a/src/keep.ts b/src/keep.ts +--- a/src/keep.ts ++++ b/src/keep.ts +@@ -1 +1,2 @@ + export const keep = 1; ++export const other = 2; +`; + +describe("unsupported inputs and deletions", () => { + const snapshot = buildStructureSnapshot(input(REMOVAL_DIFF)); + + it("marks binary and non-TypeScript files as unanalyzed with an explicit reason", () => { + const markdown = snapshot.files.find((file) => file.path === "docs/guide.md")!; + const binary = snapshot.files.find((file) => file.path === "assets/logo.png")!; + expect(markdown.analyzed).toBe(false); + expect(markdown.reason).toBe("unsupported file type '.md': no import data"); + expect(binary.analyzed).toBe(false); + expect(binary.reason).toBe("binary content: no import data"); + expect(snapshot.edges.some((found) => found.from === "docs/guide.md")).toBe(false); + }); + + it("reports the imports of a deleted file as removed connections", () => { + const found = edge(snapshot, "src/dropped.ts", "src/keep.ts")!; + expect(found.status).toBe("removed"); + expect(found.evidence).toEqual([ + { path: "src/dropped.ts", line: 1, text: "import { keep } from \"./keep.ts\";" }, + ]); + expect(snapshot.files.find((file) => file.path === "src/dropped.ts")!.status).toBe("removed"); + }); +}); + +describe("source fidelity", () => { + it("refuses to analyze pasted code", () => { + const snapshot = buildStructureSnapshot(input("import a from \"./b.ts\";", "paste")); + expect(snapshot.files).toEqual([]); + expect(snapshot.edges).toEqual([]); + expect(snapshot.comparison.partial).toBe(true); + expect(snapshot.comparison.reasons[0]).toContain("structure analysis is unavailable"); + }); + + it("labels a pull-request snapshot as patch-only partial", () => { + const snapshot = buildStructureSnapshot({ ...input(RENAME_DIFF, "pr"), headSha: "abc123" }); + expect(snapshot.comparison.partial).toBe(true); + expect(snapshot.comparison.reasons[0]).toBe("patch-only: base and head objects are not read locally"); + expect(snapshot.edges.length).toBeGreaterThan(0); + }); + + it("states the compared endpoints for every source kind", () => { + const endpoints = (snapshot: StructureSnapshot): string => + `${snapshot.comparison.from} -> ${snapshot.comparison.to}`; + expect(endpoints(buildStructureSnapshot(input(RENAME_DIFF, "worktree")))).toBe("index -> working tree"); + expect(endpoints(buildStructureSnapshot(input(RENAME_DIFF, "staged")))).toBe("HEAD -> index"); + expect(endpoints(buildStructureSnapshot({ ...input(RENAME_DIFF, "commit"), label: "Commit 9fceb02" }))) + .toBe("first parent -> commit 9fceb02"); + expect(endpoints(buildStructureSnapshot({ ...input(RENAME_DIFF, "range"), label: "main...feature" }))) + .toBe("merge-base -> feature"); + expect(endpoints(buildStructureSnapshot({ ...input(RENAME_DIFF, "pr"), headSha: "abc123" }))) + .toBe("base -> head abc123"); + expect(endpoints(buildStructureSnapshot(input("code", "paste")))).toBe("unavailable -> unavailable"); + }); +}); + +function generatedDiff(sources: number, targets: number, perSource: number): string { + const parts: string[] = []; + for (let index = 0; index < targets; index += 1) { + parts.push(`diff --git a/src/t${index}.ts b/src/t${index}.ts\n--- a/src/t${index}.ts\n+++ b/src/t${index}.ts\n@@ -1 +1,2 @@\n export const t${index} = ${index};\n+export const extra${index} = ${index};\n`); + } + for (let index = 0; index < sources; index += 1) { + const imports = Array.from({ length: perSource }, (_unused, offset) => + `+import { t${(index + offset) % targets} } from "./t${(index + offset) % targets}.ts";`).join("\n"); + parts.push(`diff --git a/src/s${index}.ts b/src/s${index}.ts\nnew file mode 100644\n--- /dev/null\n+++ b/src/s${index}.ts\n@@ -0,0 +1,${perSource} @@\n${imports}\n`); + } + return parts.join(""); +} + +describe("resource limits and determinism", () => { + it("returns a valid truncated snapshot when the file cap is exceeded", () => { + const snapshot = buildStructureSnapshot(input(generatedDiff(150, 100, 2))); + expect(snapshot.files).toHaveLength(200); + expect(snapshot.limits.truncated).toBe(true); + expect(snapshot.limits.omitted.some((omission) => omission.reason.includes("file cap reached"))).toBe(true); + expect(snapshot.comparison.partial).toBe(true); + expect(snapshot.files.map((file) => file.path)).toEqual([...snapshot.files.map((file) => file.path)].sort()); + }); + + it("returns a valid truncated snapshot when the connection cap is exceeded", () => { + const snapshot = buildStructureSnapshot(input(generatedDiff(100, 100, 21))); + expect(snapshot.files).toHaveLength(200); + expect(snapshot.edges).toHaveLength(2000); + expect(snapshot.limits.truncated).toBe(true); + expect(snapshot.limits.omitted.some((omission) => omission.reason.includes("connection cap reached"))).toBe(true); + }); + + it("bounds evidence per edge and merges duplicate statements", () => { + const duplicates = `diff --git a/src/t.ts b/src/t.ts +--- a/src/t.ts ++++ b/src/t.ts +@@ -1 +1,2 @@ + export const t = 1; ++export const u = 2; +diff --git a/src/many.ts b/src/many.ts +new file mode 100644 +--- /dev/null ++++ b/src/many.ts +@@ -0,0 +1,6 @@ ++import { a } from "./t.ts"; ++import { b } from "./t.ts"; ++import { c } from "./t.ts"; ++import { d } from "./t.ts"; ++import { e } from "./t.ts"; ++import { f } from "./t.ts"; +`; + const snapshot = buildStructureSnapshot(input(duplicates)); + const merged = edgesFrom(snapshot, "src/many.ts"); + expect(merged).toHaveLength(1); + expect(merged[0]!.evidence).toHaveLength(4); + expect(merged[0]!.evidence.map((item) => item.line)).toEqual([1, 2, 3, 4]); + }); + + it("produces byte-identical JSON across runs", () => { + const source = input(`${RENAME_DIFF}${KINDS_DIFF}${REMOVAL_DIFF}`); + expect(JSON.stringify(buildStructureSnapshot(source))).toBe(JSON.stringify(buildStructureSnapshot(source))); + }); + + it("returns a valid snapshot for a diff whose hunk header is unusable", () => { + const snapshot = structureSnapshotFor(input("diff --git a/a.ts b/a.ts\n@@ bad header @@\n+import x from \"./y\";\n")); + expect(snapshot.protocol).toBe("rt/1"); + expect(snapshot.inputId).toBe("input-1"); + expect(snapshot.edges).toEqual([]); + expect(snapshot.comparison.reasons).toContain("malformed hunk header at diff line 2: no import data for that file"); + expect(snapshot.comparison.partial).toBe(true); + expect(snapshot.files[0]!.reason).toBe("no diff content for this file: no import data"); + }); + + it("degrades to a partial snapshot when the analyzer itself throws", () => { + const hostile = { + ...input(""), + get content(): string { throw new Error("input content unavailable"); }, + } as InputSnapshot; + const snapshot = structureSnapshotFor(hostile); + expect(snapshot.protocol).toBe("rt/1"); + expect(snapshot.edges).toEqual([]); + expect(snapshot.comparison.partial).toBe(true); + expect(snapshot.comparison.reasons[0]).toContain("structure analysis failed: expected a parsable unified diff"); + expect(snapshot.comparison.reasons[0]).toContain("input content unavailable"); + }); +}); + +function changedFile(path: string, lines: string[], header = ""): string { + const body = lines.map((line) => (line.startsWith("+") || line.startsWith("-") ? line : ` ${line}`)).join("\n"); + return `diff --git a/${path} b/${path}\n${header}--- a/${path}\n+++ b/${path}\n@@ -1,${lines.length} +1,${lines.length} @@\n${body}\n`; +} + +describe("statements the lexer must not misread", () => { + it("does not bind semicolon-free code to a later module string", () => { + const diff = changedFile("src/plain.ts", ["export const value = 1"]) + + changedFile("src/free.ts", ["export const count = 1", "import { value } from \"./plain.ts\""]); + const snapshot = buildStructureSnapshot(input(diff)); + const found = edgesFrom(snapshot, "src/free.ts"); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ kind: "import", to: "src/plain.ts", specifier: "./plain.ts" }); + expect(found[0]!.evidence[0]!.line).toBe(2); + }); + + it("accepts every import clause shape", () => { + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + + changedFile("src/shapes.ts", [ + "import plain from \"./plain.ts\";", + "import * as everything from \"./plain.ts\";", + "import defaultAndNamed, { value } from \"./plain.ts\";", + "import {", + " value as renamed,", + "} from \"./plain.ts\";", + ]); + const snapshot = buildStructureSnapshot(input(diff)); + expect(edgesFrom(snapshot, "src/shapes.ts")).toHaveLength(1); + expect(edgesFrom(snapshot, "src/shapes.ts")[0]!.evidence).toHaveLength(4); + }); + + it("keeps long import clauses and reports the ones past the work bound", () => { + const clause = (count: number): string => + `import { ${Array.from({ length: count }, (_unused, index) => `value as v${index}`).join(", ")} } from "./plain.ts";`; + const short = clause(110); + const long = clause(160); + expect(short.length).toBeGreaterThan(1500); + expect(short.length).toBeLessThan(2000); + expect(long.length).toBeGreaterThan(2000); + expect(long.length).toBeLessThan(4000); + + const kept = buildStructureSnapshot(input( + changedFile("src/plain.ts", ["export const value = 1;"]) + changedFile("src/wide-clause.ts", [`+${short}`]))); + expect(edgesFrom(kept, "src/wide-clause.ts")).toHaveLength(1); + expect(kept.limits.omitted).toEqual([]); + + const dropped = buildStructureSnapshot(input( + changedFile("src/plain.ts", ["export const value = 1;"]) + changedFile("src/huge-clause.ts", [`+${long}`]))); + expect(edgesFrom(dropped, "src/huge-clause.ts")).toEqual([]); + expect(dropped.limits.omitted).toContainEqual({ + path: "src/huge-clause.ts", + reason: "import clause longer than 2000 characters at line 1: no import data for that statement", + }); + expect(dropped.comparison.partial).toBe(true); + expect(dropped.limits.truncated).toBe(true); + }); + + it("masks regex literals in expression positions but not division", () => { + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + + changedFile("src/expressions.ts", [ + "function first() { return /import { fake } from \"\\.\\/plain.ts\"/; }", + "const second = () => /import other from \"\\.\\/plain.ts\"/;", + "const ratio = a / b / c;", + "const half = y / 2; import { value } from \"./plain.ts\";", + ]); + const snapshot = buildStructureSnapshot(input(diff)); + const found = edgesFrom(snapshot, "src/expressions.ts"); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ to: "src/plain.ts", specifier: "./plain.ts" }); + expect(found[0]!.evidence[0]!.line).toBe(4); + }); + + it("treats a keyword-named member before a slash as division", () => { + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + + changedFile("src/members.ts", [ + "const scaled = obj.return / 2; import { value } from \"./plain.ts\";", + ]); + const found = edgesFrom(buildStructureSnapshot(input(diff)), "src/members.ts"); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ to: "src/plain.ts", specifier: "./plain.ts" }); + expect(found[0]!.evidence[0]!.line).toBe(1); + }); + + it("rejects member calls separated by whitespace or newlines", () => { + const diff = changedFile("src/t.ts", ["export const t = 1;"]) + + changedFile("src/spaced.ts", [ + "const loaded = shim . require(\"./t.ts\");", + "const lazy = shim ?. import(\"./t.ts\");", + "const wrapped = shim .", + " import(\"./t.ts\");", + ]); + const snapshot = buildStructureSnapshot(input(diff)); + expect(edgesFrom(snapshot, "src/spaced.ts")).toEqual([]); + expect(snapshot.limits.omitted.filter((omission) => omission.path === "src/spaced.ts")).toEqual([]); + }); + + it("ignores imports inside regex literals and member calls", () => { + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + + changedFile("src/tricky.ts", [ + "const pattern = /import { fake } from \"\\.\\/plain.ts\"/;", + "const loaded = shim.require(\"./plain.ts\");", + "const lazy = shim.import(\"./plain.ts\");", + "const ratio = total / count / 2;", + ]); + const snapshot = buildStructureSnapshot(input(diff)); + expect(edgesFrom(snapshot, "src/tricky.ts")).toEqual([]); + expect(snapshot.limits.omitted.filter((omission) => omission.path === "src/tricky.ts")).toEqual([]); + }); + + it("keeps offsets correct when the diff contains non-BMP characters", () => { + const source = "// 🚀 launch\nimport { value } from \"./plain.ts\";"; + expect(maskLiterals(source)).toHaveLength(source.length); + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + + changedFile("src/emoji.ts", ["// 🚀 launch note", "import { value } from \"./plain.ts\";"]); + const snapshot = buildStructureSnapshot(input(diff)); + expect(edgesFrom(snapshot, "src/emoji.ts")[0]!.evidence).toEqual([ + { path: "src/emoji.ts", line: 2, text: "import { value } from \"./plain.ts\";" }, + ]); + }); + + it("anchors status on the specifier line of a multi-line statement", () => { + const diff = changedFile("src/old-target.ts", ["export const value = 1;"]) + + changedFile("src/new-target.ts", ["export const value = 1;"]) + + `diff --git a/src/multi.ts b/src/multi.ts\n--- a/src/multi.ts\n+++ b/src/multi.ts\n@@ -1,4 +1,4 @@\n import {\n value\n-} from "./old-target.ts";\n+} from "./new-target.ts";\n export const multi = value;\n`; + const snapshot = buildStructureSnapshot(input(diff)); + expect(edge(snapshot, "src/multi.ts", "src/new-target.ts")).toMatchObject({ status: "added" }); + expect(edge(snapshot, "src/multi.ts", "src/new-target.ts")!.evidence.map((item) => item.line)).toEqual([1, 3]); + expect(edge(snapshot, "src/multi.ts", "src/old-target.ts")).toMatchObject({ status: "removed" }); + expect(edge(snapshot, "src/multi.ts", "src/old-target.ts")!.evidence.map((item) => item.line)).toEqual([1, 3]); + }); + + it("reads statements from every hunk without gluing them together", () => { + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + + `diff --git a/src/hunks.ts b/src/hunks.ts\n--- a/src/hunks.ts\n+++ b/src/hunks.ts\n@@ -1,2 +1,2 @@\n export const first = 1\n export const second = 2\n@@ -40,2 +40,3 @@\n export const third = 3\n+import { value } from "./plain.ts";\n`; + const snapshot = buildStructureSnapshot(input(diff)); + const found = edgesFrom(snapshot, "src/hunks.ts"); + expect(found).toHaveLength(1); + expect(found[0]).toMatchObject({ status: "added" }); + expect(found[0]!.evidence[0]!.line).toBe(41); + }); +}); + +describe("changed-state fidelity", () => { + it("reports a binding-only change as one modified connection", () => { + const diff = changedFile("src/b.ts", ["export const b = 1;", "export const c = 2;"]) + + `diff --git a/src/a.ts b/src/a.ts\n--- a/src/a.ts\n+++ b/src/a.ts\n@@ -1,2 +1,2 @@\n-import { b } from "./b.ts";\n+import { b, c } from "./b.ts";\n export const a = 1;\n`; + const snapshot = buildStructureSnapshot(input(diff)); + const found = edgesFrom(snapshot, "src/a.ts"); + expect(found).toHaveLength(1); + expect(found[0]!.status).toBe("modified"); + expect(found[0]!.specifier).toBe("./b.ts"); + expect(found[0]!.evidence.map((item) => item.text)).toEqual([ + "import { b } from \"./b.ts\";", + "import { b, c } from \"./b.ts\";", + ]); + }); + + it("marks a file with no diff content as unanalyzed", () => { + const diff = "diff --git a/src/moved.ts b/src/elsewhere.ts\nsimilarity index 100%\nrename from src/moved.ts\nrename to src/elsewhere.ts\n"; + const snapshot = buildStructureSnapshot(input(diff)); + expect(snapshot.files[0]).toMatchObject({ + path: "src/elsewhere.ts", + status: "renamed", + renamedFrom: "src/moved.ts", + analyzed: false, + reason: "no diff content for this file: no import data", + }); + }); + + it("refuses to analyze a combined merge diff", () => { + const diff = "diff --cc src/merged.ts\nindex 1111111,2222222..3333333\n--- a/src/merged.ts\n+++ b/src/merged.ts\n@@@ -1,2 -1,2 +1,3 @@@\n++import { x } from \"./x.ts\";\n"; + const snapshot = buildStructureSnapshot(input(diff)); + expect(snapshot.files).toEqual([]); + expect(snapshot.edges).toEqual([]); + expect(snapshot.comparison.reasons).toContain( + "merge commit: combined diffs are not analyzed; pick one parent range (e.g. ^1...) and retry"); + }); +}); + +describe("specifier resolution", () => { + it("resolves an old-side specifier relative to the pre-rename path", () => { + const diff = changedFile("src/old/sib.ts", ["export const sib = 1;"]) + + `diff --git a/src/old/mod.ts b/src/new/mod.ts\nsimilarity index 80%\nrename from src/old/mod.ts\nrename to src/new/mod.ts\n--- a/src/old/mod.ts\n+++ b/src/new/mod.ts\n@@ -1,2 +1,1 @@\n-import { sib } from "./sib.ts";\n export const mod = 1;\n`; + const snapshot = buildStructureSnapshot(input(diff)); + expect(edge(snapshot, "src/new/mod.ts", "src/old/sib.ts")).toMatchObject({ status: "removed" }); + }); + + it("refuses to resolve a new-side import of a path that no longer exists", () => { + const diff = `diff --git a/src/old/mod.ts b/src/new/mod.ts\nsimilarity index 80%\nrename from src/old/mod.ts\nrename to src/new/mod.ts\n--- a/src/old/mod.ts\n+++ b/src/new/mod.ts\n@@ -1,1 +1,2 @@\n export const mod = 1;\n+export const extra = 2;\n` + + `diff --git a/src/root.ts b/src/root.ts\n--- a/src/root.ts\n+++ b/src/root.ts\n@@ -1,1 +1,2 @@\n export const root = 1;\n+import { mod } from "./old/mod.ts";\n`; + const snapshot = buildStructureSnapshot(input(diff)); + expect(edgesFrom(snapshot, "src/root.ts")).toEqual([]); + expect(snapshot.limits.omitted).toContainEqual({ + path: "src/root.ts", + reason: "specifier './old/mod.ts' at line 2 matches no changed file: no import data outside the changed set", + }); + }); + + it("resolves modern extensions and backslash separators", () => { + const diff = changedFile("src/util.mts", ["export const util = 1;"]) + + changedFile("src/legacy.cts", ["export const legacy = 1;"]) + + changedFile("src/deep/index.mts", ["export const deep = 1;"]) + + changedFile("src/user.ts", [ + "import { util } from \"./util\";", + "import { legacy } from \"./legacy\";", + "import { deep } from \"./deep\";", + "import { win } from \".\\\\util.mts\";", + ]); + const snapshot = buildStructureSnapshot(input(diff)); + expect(edgesFrom(snapshot, "src/user.ts").map((found) => found.to)).toEqual([ + "src/deep/index.mts", "src/legacy.cts", "src/util.mts", + ]); + expect(edge(snapshot, "src/user.ts", "src/util.mts")!.evidence.map((item) => item.line)).toEqual([1, 4]); + }); +}); + +describe("bounded work", () => { + it("truncates over-long lines and says so", () => { + const padding = "x".repeat(4100); + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + + changedFile("src/long.ts", [`+const pad = "${padding}"; import { value } from "./plain.ts";`]); + const snapshot = buildStructureSnapshot(input(diff)); + expect(edgesFrom(snapshot, "src/long.ts")).toEqual([]); + expect(snapshot.limits.truncated).toBe(true); + expect(snapshot.limits.omitted).toContainEqual({ + reason: "line length cap: 1 lines longer than 4000 characters were truncated; imports past that point are not analyzed", + }); + }); + + it("stops reading after the diff-line cap", () => { + const snapshot = buildStructureSnapshot(input(`${"\n".repeat(200_001)}${changedFile("src/late.ts", ["+const late = 1;"])}`)); + expect(snapshot.files).toEqual([]); + expect(snapshot.comparison.partial).toBe(true); + expect(snapshot.comparison.reasons).toContain( + "diff parsing stopped after 200000 lines: no import data for the rest of this comparison"); + }); + + it("stops after the per-file statement cap", () => { + const statements = Array.from({ length: 2001 }, (_unused, index) => `+import { value as v${index} } from "./plain.ts";`); + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + changedFile("src/huge.ts", statements); + const snapshot = buildStructureSnapshot(input(diff)); + expect(snapshot.limits.truncated).toBe(true); + expect(snapshot.limits.omitted).toContainEqual({ + path: "src/huge.ts", + reason: "statement cap reached: expected at most 2000 import statements in this file, received more; later statements are not analyzed", + }); + expect(edgesFrom(snapshot, "src/huge.ts")[0]!.evidence).toHaveLength(4); + }); + + it("marks the end of the omission list when the row cap is reached", () => { + const files = Array.from({ length: 200 }, (_unused, index) => changedFile(`src/f${index}.ts`, [ + `+import { a } from "./missing-${index}-a.ts";`, + `+import { b } from "./missing-${index}-b.ts";`, + ])).join(""); + const snapshot = buildStructureSnapshot(input(files)); + expect(snapshot.limits.omitted).toHaveLength(201); + expect(snapshot.limits.omitted.at(-1)!.reason).toBe("further omissions not listed (200 more)"); + expect(snapshot.limits.omitted.at(-1)!.path).toBeUndefined(); + }); + + it("aggregates external modules with a bounded list and a total", () => { + const externals = Array.from({ length: 10 }, (_unused, index) => `+import p${index} from "pkg-${index}";`); + const snapshot = buildStructureSnapshot(input(changedFile("src/ext.ts", externals))); + expect(snapshot.limits.omitted).toContainEqual({ + path: "src/ext.ts", + reason: "external modules (10): pkg-0, pkg-1, pkg-2, pkg-3, pkg-4, pkg-5, pkg-6, pkg-7, …", + }); + }); + + it("trims evidence deterministically and neutralizes control characters", () => { + const long = `import { ${"value, ".repeat(40)}} from "./plain.ts";`; + const diff = changedFile("src/plain.ts", ["export const value = 1;"]) + + changedFile("src/wide.ts", [`+${long}`]); + const snapshot = buildStructureSnapshot(input(diff)); + const text = edgesFrom(snapshot, "src/wide.ts")[0]!.evidence[0]!.text; + expect(text).toHaveLength(200); + expect(text).toBe(long.slice(0, 200)); + const bell = buildStructureSnapshot(input(changedFile("src/plain.ts", ["export const value = 1;"]) + + changedFile("src/ctrl.ts", [`+import { value } from "./plain.ts"; // \u0007alarm`]))); + expect(edgesFrom(bell, "src/ctrl.ts")[0]!.evidence[0]!.text).toBe( + "import { value } from \"./plain.ts\"; // \uFFFDalarm"); + }); +}); + +async function startServer(diffs: string[]) { + const home = await mkdtemp(join(tmpdir(), "review-tutor-structure-")); + temporaryRoots.push(home); + let call = 0; + const server = await startReviewTutorServer({ + cwd: "/repo", + canonicalRepo: "/repo", + models: [{ id: "provider/model", label: "Model", thinkingLevels: ["low"] }], + skillPath, + home, + runner: { run: async () => ({ answer: "" }), cancel: () => {}, shutdown: async () => {} }, + execFile: async () => ({ stdout: diffs[Math.min(call++, diffs.length - 1)]!, stderr: "" }), + }); + return server; +} + +describe("structure endpoint", () => { + it("requires the session bearer token", async () => { + const server = await startServer([RENAME_DIFF]); + try { + const response = await fetch(`http://127.0.0.1:${server.port}/api/structure`); + expect(response.status).toBe(401); + await response.body?.cancel(); + } finally { + await server.close(); + } + }); + + it("reports a typed conflict when no input is loaded and recomputes on source switch", async () => { + const server = await startServer([RENAME_DIFF, REMOVAL_DIFF]); + const headers = { Authorization: `Bearer ${server.token}`, "Content-Type": "application/json" }; + try { + const empty = await fetch(`http://127.0.0.1:${server.port}/api/structure`, { headers }); + expect(empty.status).toBe(409); + expect((await empty.json() as { error: string }).error).toContain("expected a loaded input snapshot"); + + await fetch(`http://127.0.0.1:${server.port}/api/source`, { + method: "POST", headers, body: JSON.stringify({ protocol: "rt/1", kind: "worktree" }), + }).then((response) => response.json()); + const first = await fetch(`http://127.0.0.1:${server.port}/api/structure`, { headers }); + const firstBody = await first.text(); + expect(first.status).toBe(200); + expect(JSON.parse(firstBody).files.map((file: { path: string }) => file.path)).toContain("src/renamed.ts"); + const cached = await fetch(`http://127.0.0.1:${server.port}/api/structure`, { headers }); + expect(await cached.text()).toBe(firstBody); + + await fetch(`http://127.0.0.1:${server.port}/api/source`, { + method: "POST", headers, body: JSON.stringify({ protocol: "rt/1", kind: "staged" }), + }).then((response) => response.json()); + const second = await fetch(`http://127.0.0.1:${server.port}/api/structure`, { headers }); + const secondBody = await second.json() as StructureSnapshot; + expect(secondBody.files.map((file) => file.path)).toContain("src/dropped.ts"); + expect(secondBody.inputId).not.toBe((JSON.parse(firstBody) as StructureSnapshot).inputId); + } finally { + await server.close(); + } + }); +});