From 25f026379227d2d9e9881aa2836c43c60c6580e1 Mon Sep 17 00:00:00 2001 From: Jake Bromberg Date: Sat, 30 May 2026 10:35:28 -0700 Subject: [PATCH] refactor(type-catalog): extract compareBy + writeSiblingArtifact helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulls three hand-rolled multi-key sort comparators and two parallel sibling-artifact emission blocks out of type-catalog.mjs into reusable _lib helpers. Same byte-identical output, fewer lines, single source of truth for the sibling-wrapper shape. While consolidating the writers, references.json gains the {language, name, version} extractor block that the wrapper convention requires (catalog.json and files.json already had it; references.json was the lone holdout). Additive change — consumers reading `.edges` keep working unchanged. Documented in docs/pipeline-contract.md. Closes #185 Closes #186 --- docs/pipeline-contract.md | 3 + extractors/typescript/_lib/artifacts.mjs | 26 ++++++ extractors/typescript/_lib/sort.mjs | 17 ++++ extractors/typescript/test/artifacts.test.mjs | 87 +++++++++++++++++++ extractors/typescript/test/extract.test.mjs | 2 + extractors/typescript/test/sort.test.mjs | 58 +++++++++++++ extractors/typescript/type-catalog.mjs | 51 +++++------ 7 files changed, 213 insertions(+), 31 deletions(-) create mode 100644 extractors/typescript/_lib/artifacts.mjs create mode 100644 extractors/typescript/_lib/sort.mjs create mode 100644 extractors/typescript/test/artifacts.test.mjs create mode 100644 extractors/typescript/test/sort.test.mjs diff --git a/docs/pipeline-contract.md b/docs/pipeline-contract.md index b53d068a..8fdd7c2d 100644 --- a/docs/pipeline-contract.md +++ b/docs/pipeline-contract.md @@ -207,6 +207,7 @@ When invoked with `--emit-references-graph `, the extractor writes an inve ```jsonc { "schema_version": "1.1", + "extractor": { "language": "typescript", "name": "type-catalog", "version": "0.5.0" }, "edges": [ { "from": { "package": "main", "name": "FlowsheetView" }, @@ -226,6 +227,8 @@ When invoked with `--emit-references-graph `, the extractor writes an inve Each `references[]` entry on a declaration produces one edge. Resolution prefers a same-package target; failing that, a `shared`-package target; failing that, marks the edge `resolved: false` with `to.package == from.package` (fallback for unresolved external names). Edges are deduplicated by `(from.package, from.name, to.package, to.name)` and sorted by the same key, so two runs over the same input produce byte-identical files. +The `extractor` block matches the wrapper used by every other sibling artifact (catalog, files); consumers that already read `.edges` keep working unchanged. + The artifact is the right home for inverted ("what depends on X?") queries: ```jq diff --git a/extractors/typescript/_lib/artifacts.mjs b/extractors/typescript/_lib/artifacts.mjs new file mode 100644 index 00000000..13b199c1 --- /dev/null +++ b/extractors/typescript/_lib/artifacts.mjs @@ -0,0 +1,26 @@ +// artifacts.mjs +// +// Helper for writing schema_version-wrapped sibling artifacts +// (references.json, files.json, ...). Centralizes the wrapper shape so the +// {schema_version, extractor, } envelope is identical across +// every sibling output an extractor emits. + +import { writeFileSync } from 'node:fs'; + +export function writeSiblingArtifact({ + path, + schema_version, + extractorMeta, + payloadKey, + payload, + summary, + log = (msg) => process.stderr.write(msg), +}) { + const wrapped = { + schema_version, + extractor: extractorMeta, + [payloadKey]: payload, + }; + writeFileSync(path, JSON.stringify(wrapped, null, 2)); + log(summary + '\n'); +} diff --git a/extractors/typescript/_lib/sort.mjs b/extractors/typescript/_lib/sort.mjs new file mode 100644 index 00000000..fcaf93a6 --- /dev/null +++ b/extractors/typescript/_lib/sort.mjs @@ -0,0 +1,17 @@ +// sort.mjs +// +// Tiny multi-key comparator factory. Replaces hand-rolled +// if (a.k1 !== b.k1) return a.k1 < b.k1 ? -1 : 1; +// if (a.k2 !== b.k2) return a.k2 < b.k2 ? -1 : 1; +// ... +// chains across extractor sort sites with one declarative call: +// rows.sort(compareBy((r) => r.k1, (r) => r.k2, ...)); + +export const compareBy = (...keyFns) => (a, b) => { + for (const k of keyFns) { + const av = k(a); + const bv = k(b); + if (av !== bv) return av < bv ? -1 : 1; + } + return 0; +}; diff --git a/extractors/typescript/test/artifacts.test.mjs b/extractors/typescript/test/artifacts.test.mjs new file mode 100644 index 00000000..ebf4a7e2 --- /dev/null +++ b/extractors/typescript/test/artifacts.test.mjs @@ -0,0 +1,87 @@ +// Tests for _lib/artifacts.mjs (writeSiblingArtifact wrapper writer). +// +// Run with: node --test test/artifacts.test.mjs + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { writeSiblingArtifact } from '../_lib/artifacts.mjs'; +import { mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +function withTmpDir(fn) { + const dir = mkdtempSync(join(tmpdir(), 'artifacts-test-')); + try { fn(dir); } finally { rmSync(dir, { recursive: true, force: true }); } +} + +test('writeSiblingArtifact emits {schema_version, extractor, } JSON', () => { + withTmpDir((dir) => { + const path = join(dir, 'out.json'); + const summaries = []; + writeSiblingArtifact({ + path, + schema_version: '1.1', + extractorMeta: { language: 'typescript', name: 'type-catalog', version: '0.5.0' }, + payloadKey: 'entries', + payload: [{ a: 1 }, { a: 2 }], + summary: 'Wrote 2 rows', + log: (msg) => summaries.push(msg), + }); + const got = JSON.parse(readFileSync(path, 'utf8')); + assert.deepEqual(got, { + schema_version: '1.1', + extractor: { language: 'typescript', name: 'type-catalog', version: '0.5.0' }, + entries: [{ a: 1 }, { a: 2 }], + }); + assert.deepEqual(summaries, ['Wrote 2 rows\n']); + }); +}); + +test('writeSiblingArtifact pretty-prints with 2-space indent', () => { + withTmpDir((dir) => { + const path = join(dir, 'out.json'); + writeSiblingArtifact({ + path, + schema_version: '1.0', + extractorMeta: { language: 'python', name: 'type-catalog', version: '0.1.0' }, + payloadKey: 'edges', + payload: [], + summary: 'empty', + log: () => {}, + }); + const raw = readFileSync(path, 'utf8'); + assert.match(raw, /^{\n "schema_version": "1\.0",/); + }); +}); + +test('writeSiblingArtifact preserves payloadKey ordering after extractor block', () => { + withTmpDir((dir) => { + const path = join(dir, 'out.json'); + writeSiblingArtifact({ + path, + schema_version: '1.1', + extractorMeta: { language: 'typescript', name: 'type-catalog', version: '0.5.0' }, + payloadKey: 'edges', + payload: [], + summary: 'noop', + log: () => {}, + }); + const keys = Object.keys(JSON.parse(readFileSync(path, 'utf8'))); + assert.deepEqual(keys, ['schema_version', 'extractor', 'edges']); + }); +}); + +test('writeSiblingArtifact defaults log to process.stderr.write when omitted', () => { + withTmpDir((dir) => { + const path = join(dir, 'out.json'); + writeSiblingArtifact({ + path, + schema_version: '1.1', + extractorMeta: { language: 'typescript', name: 'type-catalog', version: '0.5.0' }, + payloadKey: 'entries', + payload: [], + summary: 'silent ok', + }); + assert.ok(readFileSync(path, 'utf8').length > 0); + }); +}); diff --git a/extractors/typescript/test/extract.test.mjs b/extractors/typescript/test/extract.test.mjs index c3c9a83a..dfa62b7b 100644 --- a/extractors/typescript/test/extract.test.mjs +++ b/extractors/typescript/test/extract.test.mjs @@ -238,6 +238,8 @@ test('--emit-references-graph writes a sibling references.json with sorted, dedu assert.ok(existsSync(refsPath), 'references.json should be created'); const refs = JSON.parse(readFileSync(refsPath, 'utf8')); assert.equal(refs.schema_version, '1.1'); + assert.deepEqual(refs.extractor, { language: 'typescript', name: 'type-catalog', version: refs.extractor.version }); + assert.match(refs.extractor.version, /^\d+\.\d+\.\d+$/, 'extractor.version must be semver-shaped'); assert.ok(Array.isArray(refs.edges)); assert.ok(refs.edges.length > 0, 'expected at least one edge'); for (const edge of refs.edges) { diff --git a/extractors/typescript/test/sort.test.mjs b/extractors/typescript/test/sort.test.mjs new file mode 100644 index 00000000..9f2f53af --- /dev/null +++ b/extractors/typescript/test/sort.test.mjs @@ -0,0 +1,58 @@ +// Tests for _lib/sort.mjs (compareBy multi-key comparator factory). +// +// Run with: node --test test/sort.test.mjs + +import { test } from 'node:test'; +import assert from 'node:assert/strict'; +import { compareBy } from '../_lib/sort.mjs'; + +test('compareBy with a single key sorts by that key ascending', () => { + const rows = [{ k: 'c' }, { k: 'a' }, { k: 'b' }]; + rows.sort(compareBy((r) => r.k)); + assert.deepEqual(rows.map((r) => r.k), ['a', 'b', 'c']); +}); + +test('compareBy breaks ties using subsequent keys', () => { + const rows = [ + { pkg: 'main', path: 'b.ts' }, + { pkg: 'main', path: 'a.ts' }, + { pkg: 'shared', path: 'a.ts' }, + ]; + rows.sort(compareBy((r) => r.pkg, (r) => r.path)); + assert.deepEqual(rows, [ + { pkg: 'main', path: 'a.ts' }, + { pkg: 'main', path: 'b.ts' }, + { pkg: 'shared', path: 'a.ts' }, + ]); +}); + +test('compareBy supports numeric keys via subtraction-equivalent ordering', () => { + const rows = [{ line: 30 }, { line: 5 }, { line: 12 }]; + rows.sort(compareBy((r) => r.line)); + assert.deepEqual(rows.map((r) => r.line), [5, 12, 30]); +}); + +test('compareBy returns 0 when all keys equal (sort stays stable in V8)', () => { + const rows = [ + { pkg: 'main', path: 'a.ts', tag: 'first' }, + { pkg: 'main', path: 'a.ts', tag: 'second' }, + ]; + rows.sort(compareBy((r) => r.pkg, (r) => r.path)); + assert.deepEqual(rows.map((r) => r.tag), ['first', 'second']); +}); + +test('compareBy supports nested-path key functions', () => { + const rows = [ + { from: { package: 'main', name: 'B' } }, + { from: { package: 'main', name: 'A' } }, + { from: { package: 'shared', name: 'X' } }, + ]; + rows.sort(compareBy((r) => r.from.package, (r) => r.from.name)); + assert.deepEqual(rows.map((r) => r.from.name), ['A', 'B', 'X']); +}); + +test('compareBy with no key functions is a no-op comparator (everything ties)', () => { + const rows = [{ x: 3 }, { x: 1 }, { x: 2 }]; + rows.sort(compareBy()); + assert.deepEqual(rows.map((r) => r.x), [3, 1, 2]); +}); diff --git a/extractors/typescript/type-catalog.mjs b/extractors/typescript/type-catalog.mjs index 8e5b2a6c..f7c6700f 100644 --- a/extractors/typescript/type-catalog.mjs +++ b/extractors/typescript/type-catalog.mjs @@ -22,6 +22,8 @@ import { refsForDecl, } from './_lib/references.mjs'; import { isTestPath } from './_lib/paths.mjs'; +import { compareBy } from './_lib/sort.mjs'; +import { writeSiblingArtifact } from './_lib/artifacts.mjs'; const EXTRACTOR_DIR = dirname(fileURLToPath(import.meta.url)); const EXTRACTOR_VERSION = JSON.parse(readFileSync(join(EXTRACTOR_DIR, 'package.json'), 'utf8')).version; @@ -581,12 +583,7 @@ function extractFromFile(filePath, pkgName, pkgRoot, collectImports) { const { package: pkg, path: rpath } = resolveImportSpec(r.spec, filePath); return { package: pkg, path: rpath, type_only: r.type_only, kind: r.kind, line: r.line }; }); - imports.sort((a, b) => { - if (a.package !== b.package) return a.package < b.package ? -1 : 1; - if (a.path !== b.path) return a.path < b.path ? -1 : 1; - if (a.kind !== b.kind) return a.kind < b.kind ? -1 : 1; - return a.line - b.line; - }); + imports.sort(compareBy((r) => r.package, (r) => r.path, (r) => r.kind, (r) => r.line)); fileRow = { path: relPath, package: pkgName, @@ -800,16 +797,15 @@ if (values['emit-references-graph']) { }); } } - edges.sort((a, b) => { - if (a.from.package !== b.from.package) return a.from.package < b.from.package ? -1 : 1; - if (a.from.name !== b.from.name) return a.from.name < b.from.name ? -1 : 1; - if (a.to.package !== b.to.package) return a.to.package < b.to.package ? -1 : 1; - if (a.to.name !== b.to.name) return a.to.name < b.to.name ? -1 : 1; - return 0; + edges.sort(compareBy((e) => e.from.package, (e) => e.from.name, (e) => e.to.package, (e) => e.to.name)); + writeSiblingArtifact({ + path: values['emit-references-graph'], + schema_version: SCHEMA_VERSION, + extractorMeta: { language: 'typescript', name: 'type-catalog', version: EXTRACTOR_VERSION }, + payloadKey: 'edges', + payload: edges, + summary: `Wrote references graph (${edges.length} edges) to ${values['emit-references-graph']}`, }); - const graph = { schema_version: SCHEMA_VERSION, edges }; - writeFileSync(values['emit-references-graph'], JSON.stringify(graph, null, 2)); - process.stderr.write(`Wrote references graph (${edges.length} edges) to ${values['emit-references-graph']}\n`); } // --- Sibling files.json artifact --- @@ -818,23 +814,16 @@ if (values['emit-references-graph']) { // Documented in docs/pipeline-contract.md §files artifact. First consumer is // pipeline/queries/cross-package-backward-imports.jq. if (EMIT_FILES) { - fileEntries.sort((a, b) => { - if (a.package !== b.package) return a.package < b.package ? -1 : 1; - if (a.path !== b.path) return a.path < b.path ? -1 : 1; - return 0; - }); - const filesArtifact = { - schema_version: SCHEMA_VERSION, - extractor: { - language: 'typescript', - name: 'type-catalog', - version: EXTRACTOR_VERSION, - }, - entries: fileEntries, - }; + fileEntries.sort(compareBy((f) => f.package, (f) => f.path)); const edgeCount = fileEntries.reduce((acc, f) => acc + f.imports.length, 0); - writeFileSync(values['emit-files'], JSON.stringify(filesArtifact, null, 2)); - process.stderr.write(`Wrote files (${fileEntries.length} files, ${edgeCount} edges) to ${values['emit-files']}\n`); + writeSiblingArtifact({ + path: values['emit-files'], + schema_version: SCHEMA_VERSION, + extractorMeta: { language: 'typescript', name: 'type-catalog', version: EXTRACTOR_VERSION }, + payloadKey: 'entries', + payload: fileEntries, + summary: `Wrote files (${fileEntries.length} files, ${edgeCount} edges) to ${values['emit-files']}`, + }); } process.exit(mainFiles.length > 0 ? 0 : 1);