From 9a511daea373fe9ccf90b895bcfbf9a98ccd12b0 Mon Sep 17 00:00:00 2001 From: devlsh Date: Wed, 7 Oct 2026 16:11:30 +0200 Subject: [PATCH 1/7] perf: optimize A* frontier and cell lookups --- docs/development.md | 6 +- src/grid.ts | 71 +++++ src/index.ts | 294 +++++++++--------- src/path.ts | 18 ++ src/scoring.ts | 39 +++ src/types.ts | 6 + tests/asc.test.ts | 2 +- tests/astar.test.ts | 344 --------------------- tests/search.test.ts | 705 +++++++++++++++++++++++++++++++++++++++++++ 9 files changed, 985 insertions(+), 500 deletions(-) create mode 100644 src/grid.ts create mode 100644 src/path.ts create mode 100644 src/scoring.ts delete mode 100644 tests/astar.test.ts create mode 100644 tests/search.test.ts diff --git a/docs/development.md b/docs/development.md index ebf84f2..4d65de6 100644 --- a/docs/development.md +++ b/docs/development.md @@ -4,7 +4,7 @@ This agent-only handbook owns package responsibilities, shared authoring contrac ## Package Responsibilities -`@devlsh/astar` is an ESM TypeScript A* pathfinding library for 2D grids with elevation support. Its sole public entrypoint, built from [src/index.ts](../src/index.ts), exposes `search`, helpers, and re-exported types as consumer contracts; `SearchOptions` in [src/types.ts](../src/types.ts) owns search options. [package.json](../package.json) owns metadata, dependencies, scripts, export/import maps, and package contents; [tsdown.config.ts](../tsdown.config.ts) owns output. Edit source, not generated modules/declarations. `prepack` builds current source; it neither runs tests nor requires an additional packing-validation gate. Refresh this projection when entrypoints or ownership change. +`@devlsh/astar` is an ESM TypeScript A* pathfinding library for 2D grids with elevation support. Its sole public entrypoint, built from [src/index.ts](../src/index.ts), exposes only `search` at runtime and explicitly exports consumer types. `SearchOptions` in [src/types.ts](../src/types.ts) owns search options. [package.json](../package.json) owns metadata, dependencies, scripts, export/import maps, and package contents; [tsdown.config.ts](../tsdown.config.ts) owns output. Edit source, not generated modules/declarations. `prepack` builds current source; it neither runs tests nor requires an additional packing-validation gate. Refresh this projection when entrypoints or ownership change. [README.md](../README.md) owns public examples and human onboarding. Internal modules use canonical docs/instructions/manifests, not module-local READMEs or compatibility policy copies. Public-package READMEs may document external usage/API. Preserve contribution, security, and support routes. [LICENSE](../LICENSE) grants MIT; [CODE_OF_CONDUCT.md](../CODE_OF_CONDUCT.md) is separately CC BY-SA 4.0 licensed. @@ -96,7 +96,7 @@ Absent release-age declarations do not prove eligibility. Stop for maintainer cl Inspect manifest scripts/composition; keep related scripts/config/lock changes together. Use root scripts for repository-wide work, root-relative paths with `pnpm exec ...`, and `pnpm why ` from the consumer rather than assuming root links. Bounded fixers require explicit paths; pathless means repository-wide. Apply `pnpm lint:fix ` then `pnpm fmt `, inspect scoped changes, and repair remaining/semantic findings. Format directly when intended; narrow failures instead of blind retries. Establish operational prerequisites/recovery; target CI setup to consumer artifacts. -[CONTRIBUTING checks](../CONTRIBUTING.md#checks) own command composition/exclusions. Root typechecking emits nothing; lint builds nothing. Inspect TypeScript includes before claiming test typechecking: Vitest execution does not typecheck tests. Inspect [oxlint](../oxlint.config.ts)/[oxfmt](../oxfmt.config.ts) configs and presets for file coverage; Oxlint does not lint Markdown. [vitest.config.ts](../vitest.config.ts) owns runner configuration; tests exercise `search` and helper behavior through [src/index.ts](../src/index.ts). +[CONTRIBUTING checks](../CONTRIBUTING.md#checks) own command composition/exclusions. Root typechecking emits nothing; lint builds nothing. Inspect TypeScript includes before claiming test typechecking: Vitest execution does not typecheck tests. Inspect [oxlint](../oxlint.config.ts)/[oxfmt](../oxfmt.config.ts) configs and presets for file coverage; Oxlint does not lint Markdown. [vitest.config.ts](../vitest.config.ts) owns runner configuration. Search tests exercise [src/index.ts](../src/index.ts); the comparator test imports its internal owner, [src/scoring.ts](../src/scoring.ts). [Demo instructions](../demo/AGENTS.md) own the canvas consumer, lifecycle, and deployment packaging. The demo imports library source, not package output. [demo.yml](../.github/workflows/demo.yml) owns credential-free demo validation and main-only deployment through the `production` environment. Local configuration does not prove hosted readiness or authorize deployment. Refresh this routing when the demo owners or check composition change. @@ -116,7 +116,7 @@ Select cumulatively by change type and every affected interface. Repository comm - **Docs** - Inspect Markdown, paths/anchors, lists, and routing. Run `pnpm fmt `, `pnpm fmt:check`, and `pnpm check`. No independent code review is required for docs-only work. - **TypeScript** - Focused `pnpm lint `, `pnpm typecheck`, and established behavior checks. Cross-module changes finish with check unless blocked/outside scope; broad migrations/refactors verify each logical batch. -- **Pathfinding** - At the public seam, exercise affected grid validation, cardinal/diagonal movement, corner cutting, elevation limits, built-in/custom heuristics, illegal destinations, and null paths. [tests/astar.test.ts](../tests/astar.test.ts) owns established search behavior checks. Use `pnpm test -- tests` and full `pnpm test`; refresh this selection when search contracts change. +- **Pathfinding** - At the public seam, exercise affected grid validation, cardinal/diagonal movement, corner cutting, elevation limits, built-in/custom heuristics, illegal destinations, and null paths. [tests/search.test.ts](../tests/search.test.ts) owns established search behavior checks. Use `pnpm test -- tests` and full `pnpm test`; refresh this selection when search contracts change. - **Demo** - Add [scoped checks](../demo/AGENTS.md#checks) for affected controls, terrain and endpoint edits, search feedback, resizing, and lifecycle; library checks do not replace them. - **Packaging** - `pnpm build` for changed entrypoints, output/declarations, or built consumer imports, not default static validation. - **Benchmarks** - [CONTRIBUTING benchmarks](../CONTRIBUTING.md#benchmarks) owns native commands, fixture descriptors, capture/compare files, interpretation, and paired CI transfer limits. [bench.config.ts](../bench.config.ts) owns quick/full projects and limits benchmark discovery to root `bench/**/*.bench.ts` files. This scope excludes the nested CI base checkout without custom exclusion patterns. [bench/search.bench.ts](../bench/search.bench.ts) measures the built public export without path-correctness validation. Build current source before local benchmarks. Root typechecking includes `bench`, and `pnpm test` owns existing correctness checks. No custom runner, thresholds, correctness preflight, or production optimization belongs to benchmark work. diff --git a/src/grid.ts b/src/grid.ts new file mode 100644 index 0000000..60a6d0f --- /dev/null +++ b/src/grid.ts @@ -0,0 +1,71 @@ +import { type Neighbor, type Vector } from './types'; + +export const directions = [ + [-1, 0], + [1, 0], + [0, -1], + [0, 1], + [-1, -1], + [1, 1], + [1, -1], + [-1, 1], +] as const; + +/** + * Returns cardinal neighbors first, then optional diagonals with their two corner cells. + */ +export function neighbors(vector: Vector, diagonals = false) { + const tiles: Neighbor[] = []; + + tiles.push( + [[vector[0] - 1, vector[1]], null], + [[vector[0] + 1, vector[1]], null], + [[vector[0], vector[1] - 1], null], + [[vector[0], vector[1] + 1], null], + ); + + if (diagonals) { + tiles.push( + [ + [vector[0] - 1, vector[1] - 1], + [ + [vector[0], vector[1] - 1], + [vector[0] - 1, vector[1]], + ], + ], + [ + [vector[0] + 1, vector[1] + 1], + [ + [vector[0], vector[1] + 1], + [vector[0] + 1, vector[1]], + ], + ], + [ + [vector[0] + 1, vector[1] - 1], + [ + [vector[0], vector[1] - 1], + [vector[0] + 1, vector[1]], + ], + ], + [ + [vector[0] - 1, vector[1] + 1], + [ + [vector[0], vector[1] + 1], + [vector[0] - 1, vector[1]], + ], + ], + ); + } + + return { + tiles, + total: tiles.length, + }; +} + +/** + * Retains comma-separated identity for coordinates outside the numeric grid-key domain. + */ +export function vectorId(x: number, y: number) { + return `${x},${y}`; +} diff --git a/src/index.ts b/src/index.ts index f1d6815..aafb084 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,12 +1,7 @@ -import { type BuiltinHeuristic, type Heuristic, heuristics } from './heuristics'; -import { - type Neighbor, - type OpenTile, - type ScoreOptions, - type SearchOptions, - type TileBuilderCache, - type Vector, -} from './types'; +import { directions, neighbors, vectorId } from './grid'; +import { calculatePath } from './path'; +import { asc, score } from './scoring'; +import { type CellState, type OpenTile, type SearchOptions, type TileBuilderCache, type Vector } from './types'; export function search(options: SearchOptions) { const heuristic = options.heuristic ?? 'diagonal'; @@ -15,31 +10,67 @@ export function search(options: SearchOptions) { const diagonal = options.diagonal ?? false; // Store the found path and open/closed lists. - const closed: string[] = [vectorId(options.from)]; - const end = vectorId(options.to); + const { from } = options; + const start: Vector = [from[0], from[1]]; + const { to } = options; + const destination: Vector = [to[0], to[1]]; let path: Vector[] | null = null; let open: OpenTile[] = []; + let head = 0; + + // Custom heuristics can mutate retained vectors, so their membership must remain a live scan. + // oxlint-disable-next-line anti-slop/no-runtime-typeof -- SearchOptions permits builtin names and mutable custom callbacks. + const indexed = typeof heuristic !== 'function'; + let finite = true; // Calculate grid limits. if (!Array.isArray(options.grid)) { - throw new Error('non-array grid provided'); + throw new Error('Non-array Grid provided'); } if (options.grid.length === 0 || !Array.isArray(options.grid[0])) { - throw new Error('2 dimensional grid array required'); + throw new Error('2 dimensional Grid array required'); } + const cells = new Map(); const maxX = options.grid[0].length; const maxY = options.grid.length; + const numeric = Number.isSafeInteger(maxX * maxY); + + /** + * In-grid integer coordinates have collision-free numeric keys. Other coordinates retain string identity. + * Records are local to this search and exist only for cells that it visits or reads. + */ + function cellId(vector: Vector) { + const x = vector[0]; + const y = vector[1]; + + return numeric && Number.isInteger(x) && Number.isInteger(y) && x >= 0 && y >= 0 && x < maxX && y < maxY + ? y * maxX + x + : vectorId(x, y); + } - // Helper function to determine the make-up of a Tile object, cached in-memory. - const tileCache: Record = {}; + function state(id: number | string) { + let value = cells.get(id); + + if (!value) { + value = {}; + cells.set(id, value); + } + + return value; + } + + state(cellId(start)).closed = true; + const end = cellId(destination); + + // Helper function to determine the make-up of a Tile object, cached in-memory. function tile(vector: Vector): TileBuilderCache { - const id = vectorId(vector); + const cell = state(cellId(vector)); - if (tileCache[id]) { - return tileCache[id]; + if (cell.tile) { + return cell.tile; } const rawValue = options.grid[vector[1]]?.[vector[0]]; @@ -63,24 +94,18 @@ export function search(options: SearchOptions) { }; } - tileCache[id] = value; + cell.tile = value; return value; } // Helper function to determine legality of a Vector. - function canUse([cell, neighbors]: Neighbor, origin: Vector) { + function canUse(cell: Vector, origin: Vector) { return ( // Make sure this tile is walkable. !isIllegal(cell, origin) && // Don't use closed cells. - !closed.includes(vectorId(cell)) && - // Check the neighboring cells, if diagonal movement. - // There are no neighbors to check. - (neighbors === null || - // If we can cut corners, otherwise if the corners are legal. - cutCorners || - (!isIllegal(neighbors[0], origin, 0) && !isIllegal(neighbors[1], origin, 0))) + !cells.get(cellId(cell))?.closed ); } @@ -109,25 +134,41 @@ export function search(options: SearchOptions) { ); } - // Calculates the neighbors and their scores. + /** + * Expands neighbors in grid direction order, then restores stable score order. + * Score decreases retain the preceding frontier order for ties, rather than discovery order. + */ function traverse(from: OpenTile) { - const { tiles, total } = neighbors(from[0], diagonal); + // The root can have accessor coordinates. Custom callbacks can retain and mutate any vector. + const prepared = !indexed || from[2] === null ? neighbors(from[0], diagonal).tiles : null; + const x = prepared ? 0 : from[0][0]; + const y = prepared ? 0 : from[0][1]; + const total = diagonal ? 8 : 4; + const added: OpenTile[] = []; + let decreased = false; for (let i = 0; i < total; i++) { - const neighbor = tiles[i]; - - if (!neighbor) { - continue; - } - - const name = vectorId(neighbor[0]); + const [dx, dy] = directions[i]; + const cell: Vector = prepared ? prepared[i][0] : [x + dx, y + dy]; + const name = cellId(cell); // If the tile is usable, push it to the list. - if (canUse(neighbor, from[0])) { - const existing = open.find((item) => vectorId(item[0]) === name); + if (canUse(cell, from[0])) { + if (i >= 4 && !cutCorners) { + const corners = prepared ? prepared[i][1] : null; + + if ( + isIllegal(corners ? corners[0] : [x, y + dy], from[0], 0) || + isIllegal(corners ? corners[1] : [x + dx, y], from[0], 0) + ) { + continue; + } + } + + const existing = indexed ? cells.get(name)?.open : open.find((item) => cellId(item[0]) === name); const currentScore = score({ - current: neighbor[0], + current: cell, parent: from, goal: options.to, heuristic, @@ -135,24 +176,64 @@ export function search(options: SearchOptions) { // If it is already in the open list, but this path results in a better score. if (existing && currentScore.f < existing[1].f) { - const existingName = vectorId(existing[0]); - - open = open.map((existingTile) => { - if (vectorId(existingTile[0]) === existingName) { - existingTile[1] = currentScore; - existingTile[2] = from; - } - - return existingTile; - }); + if (indexed) { + existing[1] = currentScore; + existing[2] = from; + decreased = true; + } else { + const existingName = cellId(existing[0]); + + open = open.map((existingTile) => { + if (cellId(existingTile[0]) === existingName) { + existingTile[1] = currentScore; + existingTile[2] = from; + } + + return existingTile; + }); + } } else if (!existing) { - open.push([neighbor[0], currentScore, from]); + const entry: OpenTile = [cell, currentScore, from]; + + if (indexed) { + state(name).open = entry; + added.push(entry); + } else { + open.push(entry); + } } + + finite &&= Number.isFinite(currentScore.f); } } - // Sort the new open list by F values. - open = open.toSorted((a, b) => asc(a[1].f, b[1].f)); + if (!indexed || decreased || !finite) { + // Stable sorting uses the preceding frontier order, not discovery order, after a decrease. + open = [...open.slice(head), ...added].toSorted((a, b) => asc(a[1].f, b[1].f)); + head = 0; + } else { + for (const entry of added) { + let high = open.length; + let low = head; + + while (low < high) { + const middle = Math.floor((low + high) / 2); + + if (entry[1].f < open[middle][1].f) { + high = middle; + } else { + low = middle + 1; + } + } + + open.splice(low, 0, entry); + } + + if (head > 0 && head >= open.length / 2) { + open = open.slice(head); + head = 0; + } + } } // And start traversing from the starting position. @@ -167,20 +248,24 @@ export function search(options: SearchOptions) { ]); // Traverse the open list until it is empty. - while (open.length > 0) { - const bestScore = open.shift(); + while (open.length > head) { + const bestScore = indexed ? open[head++] : open.shift(); if (bestScore) { const [vector] = bestScore; - const name = vectorId(vector); + const name = cellId(vector); // Add this to the closed list. - closed.push(name); + const cell = state(name); + cell.closed = true; + cell.open = undefined; // Check if we're at the end. if (name === end) { path = calculatePath(bestScore); open = []; + head = 0; + continue; } @@ -192,99 +277,4 @@ export function search(options: SearchOptions) { return path; } -export function calculatePath(result: OpenTile) { - let current: OpenTile | null = result; - const path: Vector[] = []; - - while (current !== null) { - path.push(current[0]); - current = current[2]; - } - - path.reverse(); - - return path; -} - -export function neighbors(vector: Vector, diagonals = false) { - const tiles: Neighbor[] = []; - - tiles.push( - [[vector[0] - 1, vector[1]], null], - [[vector[0] + 1, vector[1]], null], - [[vector[0], vector[1] - 1], null], - [[vector[0], vector[1] + 1], null], - ); - - if (diagonals) { - tiles.push( - [ - [vector[0] - 1, vector[1] - 1], - [ - [vector[0], vector[1] - 1], - [vector[0] - 1, vector[1]], - ], - ], - [ - [vector[0] + 1, vector[1] + 1], - [ - [vector[0], vector[1] + 1], - [vector[0] + 1, vector[1]], - ], - ], - [ - [vector[0] + 1, vector[1] - 1], - [ - [vector[0], vector[1] - 1], - [vector[0] + 1, vector[1]], - ], - ], - [ - [vector[0] - 1, vector[1] + 1], - [ - [vector[0], vector[1] + 1], - [vector[0] - 1, vector[1]], - ], - ], - ); - } - - return { - tiles, - total: tiles.length, - }; -} - -function resolveHeuristic(input: BuiltinHeuristic | Heuristic): Heuristic { - // oxlint-disable-next-line anti-slop/no-runtime-typeof -- The declared union supports builtin names and custom heuristic functions. - return typeof input === 'function' ? input : heuristics[input]; -} - -export function score(options: ScoreOptions) { - const g = options.parent[1].g + 1; - const h = resolveHeuristic(options.heuristic)(options.current, options.goal); - - return { - g, - h, - f: g + h, - }; -} - -export function vectorId(vector: Vector) { - return `${vector[0]},${vector[1]}`; -} - -export function asc(a: number, b: number) { - if (a > b) { - return 1; - } - - if (a < b) { - return -1; - } - - return 0; -} - -export type * from './types'; +export type { Grid, SearchOptions, Tile, TileBuilder, Vector } from './types'; diff --git a/src/path.ts b/src/path.ts new file mode 100644 index 0000000..2569c66 --- /dev/null +++ b/src/path.ts @@ -0,0 +1,18 @@ +import { type OpenTile, type Vector } from './types'; + +/** + * Follows parent links and returns the retained vectors from the origin through the result. + */ +export function calculatePath(result: OpenTile) { + let current: OpenTile | null = result; + const path: Vector[] = []; + + while (current !== null) { + path.push(current[0]); + current = current[2]; + } + + path.reverse(); + + return path; +} diff --git a/src/scoring.ts b/src/scoring.ts new file mode 100644 index 0000000..0eddfef --- /dev/null +++ b/src/scoring.ts @@ -0,0 +1,39 @@ +import { type BuiltinHeuristic, type Heuristic, heuristics } from './heuristics'; +import { type ScoreOptions } from './types'; + +/** + * Uses a custom callback unchanged or selects the named built-in heuristic. + */ +function resolveHeuristic(input: BuiltinHeuristic | Heuristic): Heuristic { + // oxlint-disable-next-line anti-slop/no-runtime-typeof -- The declared union supports builtin names and custom heuristic functions. + return typeof input === 'function' ? input : heuristics[input]; +} + +/** + * Adds one movement step to the parent cost and evaluates the heuristic against the live goal. + */ +export function score(options: ScoreOptions) { + const g = options.parent[1].g + 1; + const h = resolveHeuristic(options.heuristic)(options.current, options.goal); + + return { + g, + h, + f: g + h, + }; +} + +/** + * Compares ascending scores and treats unordered values, including NaN, as ties. + */ +export function asc(a: number, b: number) { + if (a > b) { + return 1; + } + + if (a < b) { + return -1; + } + + return 0; +} diff --git a/src/types.ts b/src/types.ts index dfbcf4d..783fb17 100644 --- a/src/types.ts +++ b/src/types.ts @@ -22,6 +22,12 @@ export interface Score { export type OpenTile = [Vector, Score, OpenTile | null]; +export interface CellState { + tile?: TileBuilderCache; + open?: OpenTile; + closed?: boolean; +} + export interface ScoreOptions { current: Vector; parent: OpenTile; diff --git a/tests/asc.test.ts b/tests/asc.test.ts index c486422..174a3ea 100644 --- a/tests/asc.test.ts +++ b/tests/asc.test.ts @@ -1,4 +1,4 @@ -import { asc } from '../src'; +import { asc } from '../src/scoring'; describe('asc', () => { test.concurrent('should rearrange in ascending order', () => { diff --git a/tests/astar.test.ts b/tests/astar.test.ts deleted file mode 100644 index f3b8201..0000000 --- a/tests/astar.test.ts +++ /dev/null @@ -1,344 +0,0 @@ -import { search, type Grid } from '../src'; -import { makeGrid } from './grid'; - -describe('search', () => { - describe('input validation', () => { - test.concurrent('should enforce array grid input', () => { - expect(() => { - return search({ - from: [0, 0], - to: [0, 7], - // @ts-expect-error - grid: null, - }); - }).toThrow(); - }); - - test.concurrent('should enforce 2 dimensional array grid input', () => { - expect(() => { - return search({ - from: [0, 0], - to: [0, 7], - grid: [], - }); - }).toThrow(); - }); - }); - - test.concurrent('should select a route using a custom heuristic', () => { - const path = search({ - from: [0, 0], - to: [2, 0], - grid: [ - [0, 0, 0], - [0, 0, 0], - ], - heuristic: (current, goal) => { - expect(goal).toStrictEqual([2, 0]); - - return current[1] === goal[1] && current[0] !== goal[0] ? 100 : 0; - }, - }); - - expect(path).toStrictEqual([ - [0, 0], - [0, 1], - [1, 1], - [2, 1], - [2, 0], - ]); - }); - - describe('movement', () => { - test.concurrent('should pathfind vertically', () => { - const path = search({ - from: [0, 0], - to: [0, 7], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [0, 0], - [0, 1], - [0, 2], - [0, 3], - [0, 4], - [0, 5], - [0, 6], - [0, 7], - ]); - }); - - test.concurrent('should pathfind horizontally', () => { - const path = search({ - from: [0, 6], - to: [4, 6], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [0, 6], - [1, 6], - [2, 6], - [3, 6], - [4, 6], - ]); - }); - - test.concurrent('should pathfind diagonally', () => { - const path = search({ - cutCorners: false, - diagonal: true, - from: [0, 7], - to: [2, 6], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [0, 7], - [1, 6], - [2, 6], - ]); - }); - - test.concurrent('should pathfind diagonally in base directions', () => { - const path = search({ - cutCorners: false, - diagonal: false, - from: [0, 7], - to: [2, 6], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [0, 7], - [1, 7], - [1, 6], - [2, 6], - ]); - }); - - test.concurrent('should pathfind diagonally without cutting corner', () => { - const path = search({ - cutCorners: false, - diagonal: true, - from: [1, 7], - to: [2, 6], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [1, 7], - [1, 6], - [2, 6], - ]); - }); - - test.concurrent('should pathfind diagonally with cutting corner', () => { - const path = search({ - cutCorners: true, - diagonal: true, - from: [1, 7], - to: [2, 6], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [1, 7], - [2, 6], - ]); - }); - }); - - describe('elevation constraints', () => { - test.concurrent('should consider elevation when pathfinding', () => { - const path = search({ - cutCorners: false, - diagonal: true, - from: [0, 0], - to: [1, 0], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [0, 0], - [0, 1], - [0, 2], - [0, 3], - [0, 4], - [1, 4], - [1, 3], - [1, 2], - [1, 1], - [1, 0], - ]); - }); - - test.concurrent('should allow differing step heights in elevation', () => { - const path = search({ - cutCorners: false, - diagonal: true, - stepHeight: 2, - from: [0, 0], - to: [1, 0], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [0, 0], - [0, 1], - [0, 2], - [0, 3], - [1, 3], - [1, 2], - [1, 1], - [1, 0], - ]); - }); - - test.concurrent('should not cut corners over elevated tiles', () => { - const path = search({ - cutCorners: false, - diagonal: true, - from: [1, 0], - to: [0, 3], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [1, 0], - [1, 1], - [1, 2], - [1, 3], - [1, 4], - [0, 4], - [0, 3], - ]); - }); - - test.concurrent('should produce similar results with manhattan heuristic', () => { - const path = search({ - heuristic: 'manhattan', - cutCorners: false, - diagonal: true, - stepHeight: 2, - from: [0, 0], - to: [1, 0], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [0, 0], - [0, 1], - [0, 2], - [0, 3], - [1, 3], - [1, 2], - [1, 1], - [1, 0], - ]); - }); - }); - - describe('destination handling', () => { - test.concurrent('should pathfind if starting point is invalid tile', () => { - const testGrid: Grid = [ - [-1, 5, 5, -1], - [5, 5, 5, -1], - [-1, -1, -1, -1], - ]; - - const path = search({ - grid: testGrid, - from: [0, 0], - to: [2, 0], - }); - - expect(path).toStrictEqual([ - [0, 0], - [1, 0], - [2, 0], - ]); - }); - - test.concurrent('should not allow moving from illegal destination tile to larger than step elevation (lol)', () => { - const path = search({ - cutCorners: false, - diagonal: false, - from: [7, 6], - to: [7, 7], - grid: makeGrid(), - }); - - expect(path).toStrictEqual(null); - }); - - test.concurrent('should fail to pathfind when destination is illegal and invalid', () => { - const path = search({ - cutCorners: false, - diagonal: false, - from: [4, 7], - to: [6, 7], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [4, 7], - [4, 6], - [4, 5], - [4, 4], - [4, 3], - [5, 3], - [6, 3], - [6, 4], - [6, 5], - [6, 6], - [6, 7], - ]); - }); - - test.concurrent('should allow pathfinding to illegal tile if valid as destination', () => { - const path = search({ - cutCorners: false, - diagonal: false, - from: [4, 7], - to: [5, 7], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [4, 7], - [5, 7], - ]); - }); - - test.concurrent('should allow pathfinding to illegal tile if valid as destination and correctly go over pathing', () => { - const path = search({ - cutCorners: false, - diagonal: false, - from: [6, 2], - to: [7, 2], - grid: makeGrid(), - }); - - expect(path).toStrictEqual([ - [6, 2], - [6, 3], - [7, 3], - [7, 2], - ]); - }); - - test.concurrent('should not pathfind to illegal tile that is valid as a destination but above step limit', () => { - const path = search({ - cutCorners: false, - diagonal: false, - from: [7, 1], - to: [7, 0], - grid: makeGrid(), - }); - - expect(path).toStrictEqual(null); - }); - }); -}); diff --git a/tests/search.test.ts b/tests/search.test.ts new file mode 100644 index 0000000..d8d851b --- /dev/null +++ b/tests/search.test.ts @@ -0,0 +1,705 @@ +import { search, type Grid, type Vector } from '../src'; +import { makeGrid } from './grid'; + +describe('search', () => { + describe('input validation', () => { + test.concurrent('should enforce array grid input', () => { + expect(() => { + return search({ + from: [0, 0], + to: [0, 7], + // @ts-expect-error + grid: null, + }); + }).toThrow(); + }); + + test.concurrent('should enforce 2 dimensional array grid input', () => { + expect(() => { + return search({ + from: [0, 0], + to: [0, 7], + grid: [], + }); + }).toThrow(); + }); + }); + + test.concurrent('should select a route using a custom heuristic', () => { + const path = search({ + from: [0, 0], + to: [2, 0], + grid: [ + [0, 0, 0], + [0, 0, 0], + ], + heuristic: (current, goal) => { + expect(goal).toStrictEqual([2, 0]); + + return current[1] === goal[1] && current[0] !== goal[0] ? 100 : 0; + }, + }); + + expect(path).toStrictEqual([ + [0, 0], + [0, 1], + [1, 1], + [2, 1], + [2, 0], + ]); + }); + + describe('search compatibility', () => { + test.concurrent('retains frontier order on equal scores', () => { + expect( + search({ + grid: [ + [0, 0, 0, 0], + [0, -1, 0, 0], + [0, 0, 0, 0], + [0, 0, 0, 0], + ], + from: [0, 0], + to: [3, 3], + }), + ).toStrictEqual([ + [0, 0], + [1, 0], + [2, 0], + [2, 1], + [2, 2], + [3, 2], + [3, 3], + ]); + }); + + test.concurrent('retains the preceding frontier order after score decreases', () => { + let seed = 248; + + const grid = Array.from({ length: 12 }, () => + Array.from({ length: 12 }, () => { + seed = (Math.imul(seed, 1664525) + 1013904223) >>> 0; + + return seed / 0x1_0000_0000 < 0.3 ? -1 : 0; + }), + ); + + grid[0][0] = 0; + grid[11][11] = 0; + + expect( + search({ + grid, + from: [0, 0], + to: [11, 11], + diagonal: true, + cutCorners: false, + heuristic: 'manhattan', + }), + ).toStrictEqual([ + [0, 0], + [1, 1], + [2, 2], + [3, 3], + [4, 4], + [4, 5], + [5, 6], + [6, 5], + [7, 4], + [8, 4], + [9, 4], + [10, 4], + [11, 4], + [11, 5], + [11, 6], + [11, 7], + [11, 8], + [11, 9], + [11, 10], + [11, 11], + ]); + }); + + test.concurrent('takes fresh snapshots in each search on mixed mutable grids', () => { + const grid: Grid = [ + [0, { elevation: 0 }, 0], + [0, 0, 0], + ]; + + expect( + search({ + grid, + from: [0, 0], + to: [2, 0], + }), + ).toStrictEqual([ + [0, 0], + [1, 0], + [2, 0], + ]); + + grid[0][1] = { + elevation: 0, + isLegal: false, + }; + + expect( + search({ + grid, + from: [0, 0], + to: [2, 0], + }), + ).toStrictEqual([ + [0, 0], + [0, 1], + [1, 1], + [2, 1], + [2, 0], + ]); + + grid[1][1] = -1; + + expect( + search({ + grid, + from: [0, 0], + to: [2, 0], + }), + ).toBeNull(); + }); + + test.concurrent('keeps lazy tile reads and reports reachable ragged cells', () => { + expect(() => + search({ + grid: [[0, 0], []], + from: [0, 0], + to: [1, 0], + }), + ).toThrow('Grid value is undefined'); + + expect( + search({ + grid: [ + [0, 0, -1], + [0, 0], + ], + from: [0, 0], + to: [1, 0], + }), + ).toStrictEqual([ + [0, 0], + [1, 0], + ]); + }); + + test.concurrent('captures endpoint identity before grid access without extra coordinate reads', () => { + const from: Vector = [0, 0]; + const to: Vector = [2, 0]; + + const reads: string[] = []; + let initialReads: string[] | undefined; + let targetX = 2; + + Object.defineProperties(from, { + 0: { + get: () => { + reads.push('from.x'); + + return 0; + }, + }, + 1: { + get: () => { + reads.push('from.y'); + + return 0; + }, + }, + }); + + Object.defineProperties(to, { + 0: { + get: () => { + reads.push('to.x'); + + return targetX; + }, + }, + 1: { + get: () => { + reads.push('to.y'); + + return 0; + }, + }, + }); + + const path = search({ + get from() { + reads.push('from'); + + return from; + }, + get to() { + reads.push('to'); + + return to; + }, + get grid() { + initialReads ??= [...reads]; + targetX = 1; + + return [[0, 0, 0]]; + }, + }); + + expect(initialReads).toStrictEqual(['from', 'from.x', 'from.y', 'to', 'to.x', 'to.y']); + expect(path).toStrictEqual([ + [0, 0], + [1, 0], + [2, 0], + ]); + }); + + test.concurrent('keeps callback vectors independent and supports reentrant searches', () => { + const retained: Vector[] = []; + let nested = false; + + const path = search({ + grid: [ + [0, 0, 0], + [0, 0, 0], + ], + from: [0, 0], + to: [2, 0], + heuristic: (current) => { + retained.push(current); + + if (!nested) { + nested = true; + expect( + search({ + grid: [[0, 0]], + from: [0, 0], + to: [1, 0], + }), + ).toStrictEqual([ + [0, 0], + [1, 0], + ]); + } + + return 0; + }, + }); + + expect(path).toStrictEqual([ + [0, 0], + [1, 0], + [2, 0], + ]); + expect(new Set(retained).size).toBe(retained.length); + expect(retained[0]).toStrictEqual([1, 0]); + expect(retained[1]).toStrictEqual([0, 1]); + }); + + test.concurrent('prepares neighbors before callbacks can change the origin', () => { + const from: Vector = [0, 0]; + const seen: Vector[] = []; + + const path = search({ + grid: [ + [0, 0, 0], + [0, 0, 0], + ], + from, + to: [2, 0], + heuristic: (current) => { + seen.push([...current]); + from[0] = 1; + + return 0; + }, + }); + + expect(path).toStrictEqual([ + [1, 0], + [1, 0], + [2, 0], + ]); + expect(seen.slice(0, 2)).toStrictEqual([ + [1, 0], + [0, 1], + ]); + }); + + test.concurrent('preserves custom mutation, exceptions, and nonfinite ordering', () => { + expect( + search({ + grid: [[0, 0, 0]], + from: [0, 0], + to: [2, 0], + heuristic: (current) => { + current[0] = 2; + + return 0; + }, + }), + ).toStrictEqual([ + [0, 0], + [2, 0], + ]); + + const failure = new Error('heuristic failure'); + + expect(() => + search({ + grid: [[0, 0]], + from: [0, 0], + to: [1, 0], + heuristic: () => { + throw failure; + }, + }), + ).toThrow(failure); + + for (const value of [Number.NaN, Infinity, -Infinity]) { + expect( + search({ + grid: [[0, 0]], + from: [0, 0], + to: [1, 0], + heuristic: () => value, + }), + ).toStrictEqual([ + [0, 0], + [1, 0], + ]); + } + }); + + test.concurrent('snapshots object tiles once at first access', () => { + let elevation = 0; + let reads = 0; + + const middle = { + get elevation() { + reads++; + + return elevation; + }, + }; + + const path = search({ + grid: [[0, middle, 0]], + from: [0, 0], + to: [2, 0], + heuristic: () => { + elevation = 10; + + return 0; + }, + }); + + expect(path).toStrictEqual([ + [0, 0], + [1, 0], + [2, 0], + ]); + expect(reads).toBe(2); + }); + }); + + describe('movement', () => { + test.concurrent('should pathfind vertically', () => { + const path = search({ + from: [0, 0], + to: [0, 7], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [0, 0], + [0, 1], + [0, 2], + [0, 3], + [0, 4], + [0, 5], + [0, 6], + [0, 7], + ]); + }); + + test.concurrent('should pathfind horizontally', () => { + const path = search({ + from: [0, 6], + to: [4, 6], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [0, 6], + [1, 6], + [2, 6], + [3, 6], + [4, 6], + ]); + }); + + test.concurrent('should pathfind diagonally', () => { + const path = search({ + cutCorners: false, + diagonal: true, + from: [0, 7], + to: [2, 6], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [0, 7], + [1, 6], + [2, 6], + ]); + }); + + test.concurrent('should pathfind diagonally in base directions', () => { + const path = search({ + cutCorners: false, + diagonal: false, + from: [0, 7], + to: [2, 6], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [0, 7], + [1, 7], + [1, 6], + [2, 6], + ]); + }); + + test.concurrent('should pathfind diagonally without cutting corner', () => { + const path = search({ + cutCorners: false, + diagonal: true, + from: [1, 7], + to: [2, 6], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [1, 7], + [1, 6], + [2, 6], + ]); + }); + + test.concurrent('should pathfind diagonally with cutting corner', () => { + const path = search({ + cutCorners: true, + diagonal: true, + from: [1, 7], + to: [2, 6], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [1, 7], + [2, 6], + ]); + }); + }); + + describe('elevation constraints', () => { + test.concurrent('should consider elevation when pathfinding', () => { + const path = search({ + cutCorners: false, + diagonal: true, + from: [0, 0], + to: [1, 0], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [0, 0], + [0, 1], + [0, 2], + [0, 3], + [0, 4], + [1, 4], + [1, 3], + [1, 2], + [1, 1], + [1, 0], + ]); + }); + + test.concurrent('should allow differing step heights in elevation', () => { + const path = search({ + cutCorners: false, + diagonal: true, + stepHeight: 2, + from: [0, 0], + to: [1, 0], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [0, 0], + [0, 1], + [0, 2], + [0, 3], + [1, 3], + [1, 2], + [1, 1], + [1, 0], + ]); + }); + + test.concurrent('should not cut corners over elevated tiles', () => { + const path = search({ + cutCorners: false, + diagonal: true, + from: [1, 0], + to: [0, 3], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [1, 0], + [1, 1], + [1, 2], + [1, 3], + [1, 4], + [0, 4], + [0, 3], + ]); + }); + + test.concurrent('should produce similar results with manhattan heuristic', () => { + const path = search({ + heuristic: 'manhattan', + cutCorners: false, + diagonal: true, + stepHeight: 2, + from: [0, 0], + to: [1, 0], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [0, 0], + [0, 1], + [0, 2], + [0, 3], + [1, 3], + [1, 2], + [1, 1], + [1, 0], + ]); + }); + }); + + describe('destination handling', () => { + test.concurrent('should pathfind if starting point is invalid tile', () => { + const testGrid: Grid = [ + [-1, 5, 5, -1], + [5, 5, 5, -1], + [-1, -1, -1, -1], + ]; + + const path = search({ + grid: testGrid, + from: [0, 0], + to: [2, 0], + }); + + expect(path).toStrictEqual([ + [0, 0], + [1, 0], + [2, 0], + ]); + }); + + test.concurrent('should not allow moving from illegal destination tile to larger than step elevation (lol)', () => { + const path = search({ + cutCorners: false, + diagonal: false, + from: [7, 6], + to: [7, 7], + grid: makeGrid(), + }); + + expect(path).toStrictEqual(null); + }); + + test.concurrent('should fail to pathfind when destination is illegal and invalid', () => { + const path = search({ + cutCorners: false, + diagonal: false, + from: [4, 7], + to: [6, 7], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [4, 7], + [4, 6], + [4, 5], + [4, 4], + [4, 3], + [5, 3], + [6, 3], + [6, 4], + [6, 5], + [6, 6], + [6, 7], + ]); + }); + + test.concurrent('should allow pathfinding to illegal tile if valid as destination', () => { + const path = search({ + cutCorners: false, + diagonal: false, + from: [4, 7], + to: [5, 7], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [4, 7], + [5, 7], + ]); + }); + + test.concurrent('should allow pathfinding to illegal tile if valid as destination and correctly go over pathing', () => { + const path = search({ + cutCorners: false, + diagonal: false, + from: [6, 2], + to: [7, 2], + grid: makeGrid(), + }); + + expect(path).toStrictEqual([ + [6, 2], + [6, 3], + [7, 3], + [7, 2], + ]); + }); + + test.concurrent('should not pathfind to illegal tile that is valid as a destination but above step limit', () => { + const path = search({ + cutCorners: false, + diagonal: false, + from: [7, 1], + to: [7, 0], + grid: makeGrid(), + }); + + expect(path).toStrictEqual(null); + }); + }); +}); From aaa29469b6c0d7a627d4900804f5e64eb603909d Mon Sep 17 00:00:00 2001 From: devlsh Date: Wed, 7 Oct 2026 19:48:18 +0200 Subject: [PATCH 2/7] feat: compare benchmarks against an external baseline --- CONTRIBUTING.md | 22 ++++++++++++++++++--- bench.config.ts | 46 ++++++++++++++++++++++++++----------------- bench/baseline.ts | 17 ++++++++++++++++ bench/search.bench.ts | 13 +++++++++++- 4 files changed, 76 insertions(+), 22 deletions(-) create mode 100644 bench/baseline.ts diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 927a558..4efad97 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -87,7 +87,7 @@ Domain files in `bench/fixtures/` declare grid factories and search options. The Run the quick cases with an adjacent target: ```sh -pnpm bench:quick --reporter=default -t 'Adjacent target' +pnpm bench:quick -t 'Adjacent target' ``` Run `pnpm test` for the existing library correctness tests. These tests use the source public export and need no build. @@ -107,11 +107,27 @@ Capture a local reference only when you intend to create or replace it. Then dis ```sh pnpm bench:quick --mode capture -pnpm bench:quick --mode compare --reporter=default +pnpm bench:quick --mode compare ``` Use `bench:full` instead of `bench:quick` for a full reference. +To compare against a separate checkout, first build and capture the reference in that checkout: + +```sh +pnpm build +pnpm bench:quick --mode capture +pnpm bench:full --mode capture +``` + +Then enter your candidate worktree. Build its current source and compare against the reference checkout: + +```sh +pnpm build +BENCH_BASELINE=/path/to/baseline/repo pnpm bench:quick --mode compare +BENCH_BASELINE=/path/to/baseline/repo pnpm bench:full --mode compare +``` + When full sampling settings change, capture a fresh local full reference before comparison, even when workload IDs stay unchanged. Native reference files do not record sampling settings, so comparison cannot detect this mismatch. Quick references need no replacement for a full-only sampling change. CI captures a fresh base reference with the head harness for each paired run. @@ -119,7 +135,7 @@ Native reference files do not record sampling settings, so comparison cannot det Check warmup sensitivity with the same unchanged build: ```sh -pnpm bench:quick --mode double-warmup --reporter=default +pnpm bench:quick --mode double-warmup ``` This mode doubles warmup time without changing measurement floors or reference files. diff --git a/bench.config.ts b/bench.config.ts index b5b1bd0..ca06d2e 100644 --- a/bench.config.ts +++ b/bench.config.ts @@ -1,21 +1,31 @@ +import { resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { defineConfig } from 'vitest/config'; -export default defineConfig(({ mode }) => ({ - test: { - benchmark: { include: ['bench/**/*.bench.ts'] }, - fileParallelism: false, - maxWorkers: 1, - projects: (['quick', 'full'] as const).map((name) => ({ - extends: true, - test: { - name, - pool: 'forks', - testTimeout: 0, - provide: { - benchmarkSuite: name, - benchmarkMode: mode, +export default defineConfig(({ mode }) => { + const baseline = mode === 'compare' ? process.env.BENCH_BASELINE : undefined; + + return { + test: { + benchmark: { include: ['bench/**/*.bench.ts'] }, + fileParallelism: false, + maxWorkers: 1, + projects: (['quick', 'full'] as const).map((name) => ({ + extends: true, + test: { + name, + pool: 'forks', + testTimeout: 0, + provide: { + benchmarkSuite: name, + benchmarkMode: mode, + benchmarkBaseline: + baseline === undefined || baseline === '' + ? null + : resolve(fileURLToPath(new URL('.', import.meta.url)), baseline, '.vitest/astar'), + }, }, - }, - })), - }, -})); + })), + }, + }; +}); diff --git a/bench/baseline.ts b/bench/baseline.ts new file mode 100644 index 0000000..932c5c2 --- /dev/null +++ b/bench/baseline.ts @@ -0,0 +1,17 @@ +import { readFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { type BaselineData, type BenchFromSource } from 'vitest'; +import { type Suite } from './fixtures'; + +export function baselineFrom(directory: string, suite: Suite, id: string): BenchFromSource { + const path = join(directory, suite, `${id}.json`); + + return async () => { + try { + // SAFETY: References come from native Vitest capture, not arbitrary JSON. + return JSON.parse(await readFile(path, 'utf8')) as BaselineData; + } catch (error) { + throw new Error(`Cannot read BENCH_BASELINE reference "${path}".`, { cause: error }); + } + }; +} diff --git a/bench/search.bench.ts b/bench/search.bench.ts index 00897de..981d597 100644 --- a/bench/search.bench.ts +++ b/bench/search.bench.ts @@ -1,11 +1,13 @@ import { search as publicSearch } from '@devlsh/astar'; import { inject, test } from 'vitest'; +import { baselineFrom } from './baseline'; import { fixtures, select, type Suite } from './fixtures'; declare module 'vitest' { interface ProvidedContext { benchmarkSuite: Suite; benchmarkMode: string; + benchmarkBaseline: string | null; } } @@ -13,6 +15,8 @@ const selection = inject('benchmarkSuite'); const mode = inject('benchmarkMode'); +const baselineDirectory = inject('benchmarkBaseline'); + const search = publicSearch; const config = { @@ -54,7 +58,14 @@ for (const workload of workloads) { ); await (mode === 'compare' - ? bench.compare(bench.from('baseline (4 searches)', baseline), current, config) + ? bench.compare( + bench.from( + 'baseline (4 searches)', + baselineDirectory === null ? baseline : baselineFrom(baselineDirectory, selection, workload.id), + ), + current, + config, + ) : current.run(config)); }); } From 1b883d0c51217a5454baa0dd157e885a1ee0c2c2 Mon Sep 17 00:00:00 2001 From: devlsh Date: Thu, 8 Oct 2026 15:03:07 +0200 Subject: [PATCH 3/7] feat: publish benchmark comparisons on pull requests --- .github/actions/benchmark-capture/action.yml | 77 +++++++++ .github/actions/benchmark-report/action.yml | 33 ++++ .github/scripts/benchmark-report.mjs | 161 +++++++++++++++++++ .github/workflows/benchmark.yml | 62 ++++--- bench.config.ts | 2 +- bench/search.bench.ts | 4 +- docs/development.md | 12 +- 7 files changed, 321 insertions(+), 30 deletions(-) create mode 100644 .github/actions/benchmark-capture/action.yml create mode 100644 .github/actions/benchmark-report/action.yml create mode 100644 .github/scripts/benchmark-report.mjs diff --git a/.github/actions/benchmark-capture/action.yml b/.github/actions/benchmark-capture/action.yml new file mode 100644 index 0000000..e48f42f --- /dev/null +++ b/.github/actions/benchmark-capture/action.yml @@ -0,0 +1,77 @@ +name: "๐Ÿ“Š capture benchmark" +description: "Capture benchmarks for the base and head." + +inputs: + base-repository: + description: "Repository" + required: true + base-sha: + description: "Base commit SHA" + required: true + head-sha: + description: "Head commit SHA" + required: true + +outputs: + artifact-id: + description: "Results Artifact ID" + value: ${{ steps.results.outputs.artifact-id }} + +runs: + using: composite + steps: + - name: "๐Ÿ“ฅ check out base" + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + repository: ${{ inputs.base-repository }} + ref: ${{ inputs.base-sha }} + path: benchmark-base + persist-credentials: false + + - name: "๐Ÿ“ฅ install base dependencies" + shell: bash + working-directory: benchmark-base + run: pnpm install --frozen-lockfile + + - name: "๐Ÿ“ฆ build base" + shell: bash + working-directory: benchmark-base + run: pnpm build + + - name: "๐Ÿ“‚ stage base output" + shell: bash + run: | + mkdir -p dist + cp -R benchmark-base/dist/. dist/ + + - name: "๐Ÿ“Š capture base benchmark" + shell: bash + run: pnpm bench:full --mode capture --reporter=default + + - name: "๐Ÿ“ฆ build head" + shell: bash + run: pnpm build + + - name: "๐Ÿ“Š compare benchmark" + shell: bash + run: pnpm bench:full --mode compare --reporter=default --reporter=github-actions + + - name: "๐Ÿ“‚ collect results" + shell: bash + env: + BASE_SHA: ${{ inputs.base-sha }} + HEAD_SHA: ${{ inputs.head-sha }} + run: | + mkdir -p benchmark-results/full/current + cp .vitest/benchmarks/full/*.json benchmark-results/full/ + cp .vitest/benchmarks/full/current/*.json benchmark-results/full/current/ + node -e 'require("node:fs").writeFileSync("benchmark-results/measurement.json", JSON.stringify({base: process.env.BASE_SHA, head: process.env.HEAD_SHA}))' + + - name: "๐Ÿ“ค upload results" + id: results + uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 + with: + name: benchmark-results-${{ github.run_attempt }} + path: benchmark-results/ + if-no-files-found: error + retention-days: 1 diff --git a/.github/actions/benchmark-report/action.yml b/.github/actions/benchmark-report/action.yml new file mode 100644 index 0000000..77c13f4 --- /dev/null +++ b/.github/actions/benchmark-report/action.yml @@ -0,0 +1,33 @@ +name: "๐Ÿ“ report benchmarks" +description: "Download benchmark results and publish the report" + +inputs: + artifact-id: + description: "Results Artifact ID" + required: true + base-sha: + description: "Base commit SHA" + required: true + head-sha: + description: "Head commit SHA" + required: true + +runs: + using: composite + steps: + - name: "๐Ÿ“ฅ download results" + uses: actions/download-artifact@9000827ccba6bdab643e8b6fd33ac0654aef8333 # v8.0.2 + with: + artifact-ids: ${{ inputs.artifact-id }} + path: ${{ runner.temp }}/benchmark-results + + - name: "๐Ÿ“ publish results" + uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + BASE_SHA: ${{ inputs.base-sha }} + HEAD_SHA: ${{ inputs.head-sha }} + RESULTS: ${{ runner.temp }}/benchmark-results + with: + script: | + const { publish } = await import(`${process.env.GITHUB_WORKSPACE}/.github/scripts/benchmark-report.mjs`); + await publish({ github, context, core }); diff --git a/.github/scripts/benchmark-report.mjs b/.github/scripts/benchmark-report.mjs new file mode 100644 index 0000000..2740ba0 --- /dev/null +++ b/.github/scripts/benchmark-report.mjs @@ -0,0 +1,161 @@ +/* oxlint-disable typescript/no-unsafe-member-access, typescript/no-unsafe-return */ +// Native github-script API objects and parsed JSON have no SDK types here. + +import { readFile, readdir } from 'node:fs/promises'; +import path from 'node:path'; + +export async function publish({ github, context, core }) { + const source = { + pr: context.payload.pull_request.number, + run: context.runId, + number: context.runNumber, + attempt: Number(process.env.GITHUB_RUN_ATTEMPT), + base: process.env.BASE_SHA, + head: process.env.HEAD_SHA, + }; + + const repository = context.repo; + + const skip = (reason) => { + core.notice(`Benchmark report skipped: ${reason}`); + }; + + let rows; + + try { + const measurement = await readJson(path.join(process.env.RESULTS, 'measurement.json')); + + if (measurement?.base !== source.base || measurement?.head !== source.head) { + skip('measurement SHAs do not match the producer base and head'); + + return; + } + + const baseDirectory = path.join(process.env.RESULTS, 'full'); + const headDirectory = path.join(baseDirectory, 'current'); + const baseEntries = await readdir(baseDirectory); + const headEntries = await readdir(headDirectory); + const baseFiles = baseEntries.filter((file) => file !== 'current').toSorted((a, b) => a.localeCompare(b)); + const headFiles = headEntries.toSorted((a, b) => a.localeCompare(b)); + + if ( + baseFiles.length === 0 || + baseFiles.length !== headFiles.length || + baseFiles.some((file, index) => file !== headFiles[index] || !/^[a-z0-9][a-z0-9.-]{0,95}\.json$/u.test(file)) + ) { + skip('native results must contain matching JSON files with safe workload IDs'); + + return; + } + + rows = []; + + for (const file of baseFiles) { + const baseResult = await readJson(path.join(baseDirectory, file)); + const headResult = await readJson(path.join(headDirectory, file)); + const base = Number.isFinite(baseResult?.latency?.mean) ? baseResult.latency.mean / 4 : Number.NaN; + const head = Number.isFinite(headResult?.latency?.mean) ? headResult.latency.mean / 4 : Number.NaN; + const change = (head / base - 1) * 100; + + if (!Number.isFinite(base) || base <= 0 || !Number.isFinite(head) || head <= 0 || !Number.isFinite(change)) { + skip('latency means and per-search values must be finite and positive'); + + return; + } + + rows.push( + `| \`${file.slice(0, -5)}\` | ${base.toPrecision(4)} | ${head.toPrecision(4)} | ${change >= 0 ? '+' : ''}${change.toPrecision(3)}% |`, + ); + } + } catch { + skip('native result JSON is missing, unreadable, or invalid'); + + return; + } + + const marker = ''; + + const comments = await github.paginate(github.rest.issues.listComments, { + ...repository, + issue_number: source.pr, + per_page: 100, + }); + + const matching = comments.filter( + (comment) => + comment.user?.type === 'Bot' && comment.user.login === 'github-actions[bot]' && comment.body?.startsWith(marker), + ); + + if (matching.length > 1) { + skip('multiple bot report comments exist'); + + return; + } + + const comment = matching[0]; + + if (comment) { + const previous = comment.body.match(//u); + + if (!previous) { + skip('the existing report has no trusted run identity'); + + return; + } + + if ( + previous[1] === source.head && + (Number(previous[3]) > source.number || + (Number(previous[3]) === source.number && + (Number(previous[2]) !== source.run || Number(previous[4]) >= source.attempt))) + ) { + skip('an equal or newer report already exists for this head'); + + return; + } + } + + const url = `${context.serverUrl}/${repository.owner}/${repository.repo}`; + + const body = [ + marker, + ``, + '## Benchmark Comparison', + '', + '| Workload | Base, ms/search | PR, ms/search | Change |', + '| --- | ---: | ---: | ---: |', + ...rows, + '', + `[Base ${source.base}](${url}/commit/${source.base}) ยท [PR ${source.head}](${url}/commit/${source.head}) ยท [Run ${source.number}, attempt ${source.attempt}](${url}/actions/runs/${source.run}/attempts/${source.attempt})`, + ].join('\n'); + + const { data: pr } = await github.rest.pulls.get({ + ...repository, + pull_number: source.pr, + }); + + if (pr.state !== 'open' || pr.base.sha !== source.base || pr.head.sha !== source.head) { + skip('PR closed or base/head changed before publication'); + + return; + } + + // GitHub has no compare-and-swap for comments. A PR update can still race this final check and write. + await (comment + ? github.rest.issues.updateComment({ + ...repository, + comment_id: comment.id, + body, + }) + : github.rest.issues.createComment({ + ...repository, + issue_number: source.pr, + body, + })); + + await core.summary.addRaw(body).write(); +} + +async function readJson(file) { + return JSON.parse(await readFile(file, 'utf8')); +} diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 92a44d8..9aec5c8 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -15,6 +15,10 @@ jobs: name: "๐Ÿ“Š compare benchmarks" runs-on: ubuntu-24.04 timeout-minutes: 15 + outputs: + artifact-id: ${{ steps.results.outputs.artifact-id }} + base: ${{ github.event.pull_request.base.sha }} + head: ${{ github.event.pull_request.head.sha }} steps: - name: "๐Ÿ“ฅ check out head" @@ -27,32 +31,40 @@ jobs: - name: "โš™๏ธ set up node and pnpm" uses: devlsh/tools/github/setup@5a43d10d4535a7cf1368acd9313e70d417ac6235 - - name: "๐Ÿ“ฅ check out base" + - name: "๐Ÿ“Š compare benchmarks" + id: results + uses: ./.github/actions/benchmark-capture + with: + base-repository: ${{ github.event.pull_request.base.repo.full_name }} + base-sha: ${{ github.event.pull_request.base.sha }} + head-sha: ${{ github.event.pull_request.head.sha }} + + report: + name: "๐Ÿ“ report benchmarks" + needs: benchmark + if: >- + github.event.pull_request.head.repo.full_name == github.repository && + needs.benchmark.outputs.artifact-id != '' && + needs.benchmark.outputs.base != '' && + needs.benchmark.outputs.head != '' + runs-on: ubuntu-24.04 + permissions: + contents: read + issues: write + steps: + - name: "๐Ÿ“ฅ check out publisher" uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 with: - repository: ${{ github.event.pull_request.base.repo.full_name }} - ref: ${{ github.event.pull_request.base.sha }} - path: benchmark-base + ref: ${{ github.event.pull_request.head.sha }} persist-credentials: false + sparse-checkout: | + .github/actions/benchmark-report + .github/scripts/benchmark-report.mjs + sparse-checkout-cone-mode: false - - name: "๐Ÿ“ฅ install base dependencies" - working-directory: benchmark-base - run: pnpm install --frozen-lockfile - - - name: "๐Ÿ“ฆ build base" - working-directory: benchmark-base - run: pnpm build - - - name: "๐Ÿ“‚ stage base output" - run: | - mkdir -p dist - cp -R benchmark-base/dist/. dist/ - - - name: "๐Ÿ“Š capture base benchmark" - run: pnpm bench:full --mode capture --reporter=default - - - name: "๐Ÿ“ฆ build head" - run: pnpm build - - - name: "๐Ÿ“Š compare benchmark" - run: pnpm bench:full --mode compare --reporter=default --reporter=github-actions + - name: "๐Ÿ“ report benchmarks" + uses: ./.github/actions/benchmark-report + with: + artifact-id: ${{ needs.benchmark.outputs.artifact-id }} + base-sha: ${{ needs.benchmark.outputs.base }} + head-sha: ${{ needs.benchmark.outputs.head }} diff --git a/bench.config.ts b/bench.config.ts index ca06d2e..b2baffd 100644 --- a/bench.config.ts +++ b/bench.config.ts @@ -22,7 +22,7 @@ export default defineConfig(({ mode }) => { benchmarkBaseline: baseline === undefined || baseline === '' ? null - : resolve(fileURLToPath(new URL('.', import.meta.url)), baseline, '.vitest/astar'), + : resolve(fileURLToPath(new URL('.', import.meta.url)), baseline, '.vitest/benchmarks'), }, }, })), diff --git a/bench/search.bench.ts b/bench/search.bench.ts index 981d597..30a95ab 100644 --- a/bench/search.bench.ts +++ b/bench/search.bench.ts @@ -39,8 +39,8 @@ for (const workload of workloads) { let sink = 0; test(workload.name, async ({ bench }) => { - const baseline = `.vitest/astar/${selection}/${workload.id}.json`; - const candidate = `.vitest/astar/${selection}/current/${workload.id}.json`; + const baseline = `.vitest/benchmarks/${selection}/${workload.id}.json`; + const candidate = `.vitest/benchmarks/${selection}/current/${workload.id}.json`; const current = bench( 'current (4 searches)', diff --git a/docs/development.md b/docs/development.md index 4d65de6..cd60524 100644 --- a/docs/development.md +++ b/docs/development.md @@ -102,9 +102,17 @@ Inspect manifest scripts/composition; keep related scripts/config/lock changes t [validate.yml](../.github/workflows/validate.yml) owns ordinary CI validation. CI runs without Nix/devenv. Local setup remains unchanged. -The PR-only [benchmark.yml](../.github/workflows/benchmark.yml) owns paired benchmark execution, separate from validation. Its benchmark job captures the event base and compares the event head on one runner with the head harness. Each revision uses its own frozen lockfile under the head-selected toolchain. +The PR-only [benchmark.yml](../.github/workflows/benchmark.yml) owns job permissions, head checkout, toolchain setup, job dependencies, producer outputs, and local action routing, separate from validation. The [capture composite action](../.github/actions/benchmark-capture/action.yml) owns base checkout, paired native execution, result collection, and upload. It captures the event base and compares the event head on one runner with the head harness. Each revision uses its own frozen lockfile under the head-selected toolchain. -Native reference and candidate files stay in the job workspace, with no uploads or measurement history. The **๐Ÿ“Š compare benchmarks** job output shows the native comparison table. Capture uses the native default reporter. Comparison explicitly selects the native default and GitHub Actions reporters. The GitHub Actions reporter supplies failure annotations and a test summary, not benchmark metrics in the job summary. The ordinary validation job remains separate. Pins, permissions, and steps belong to the workflow, not this projection. +After successful comparison, the job uploads native reference/candidate JSON and measurement SHAs in an attempt-specific artifact with short retention. It keeps no measurement history. The **๐Ÿ“Š compare benchmarks** job output retains the native comparison table, failure annotations, and test summary. Capture uses the native default reporter. Comparison selects the native default and GitHub Actions reporters. + +The same workflow has a read-only benchmark job and a report job with comment-write permission. Reports support same-repository PRs only. Fork PR reports are unsupported. Same-repository branch authors are trusted repository collaborators who control the workflow and reporter. The [report composite action](../.github/actions/benchmark-report/action.yml) owns the artifact download and script invocation. The report job executes [benchmark-report.mjs](../.github/scripts/benchmark-report.mjs) from the exact event head SHA, without benchmark code, dependency installation, or caches. + +The report job requires successful benchmarks and non-empty artifact/base/head outputs. It downloads the exact producer artifact ID from the current run. Failed-report-only reruns skip the report if those outputs are missing. Output retention for that rerun mode is not guaranteed here. A full rerun produces a fresh artifact ID. + +The publisher requires non-empty sets of native JSON files with equal counts and the same safe workload IDs. It requires finite positive per-search means and measurement SHAs that match producer outputs. It checks that the PR remains open with the measured base/head before publication. Invalid, missing, ambiguous, detected stale, or superseded results leave the old comment unchanged. GitHub comment writes have no atomic compare-and-swap, so a PR update can race the final check and write. + +The publisher writes one table to its job summary and one bot-authored sticky comment. Rows use workload IDs, mean latency divided by four in ms/search, and signed `(head/base - 1) * 100` change. Positive change means slower. Results are advisory, not a performance gate or a statistical significance claim. The workflow, composite actions, and publisher own their executable contracts. Refresh this projection when those owners change. Reserve `.scratch/` for local development, never CI. From 8d49a008f4c34f2b30cf02c213e68ac31b58bab0 Mon Sep 17 00:00:00 2001 From: devlsh Date: Thu, 8 Oct 2026 19:01:37 +0200 Subject: [PATCH 4/7] feat: generalize benchmark comparisons and result reporting --- .github/actions/benchmark-capture/action.yml | 41 +++++++++++++------- .github/actions/benchmark-report/action.yml | 8 ++++ .github/scripts/benchmark-report.mjs | 22 +++++++---- .github/workflows/benchmark.yml | 6 +++ docs/development.md | 6 +-- 5 files changed, 60 insertions(+), 23 deletions(-) diff --git a/.github/actions/benchmark-capture/action.yml b/.github/actions/benchmark-capture/action.yml index e48f42f..d4d97c5 100644 --- a/.github/actions/benchmark-capture/action.yml +++ b/.github/actions/benchmark-capture/action.yml @@ -11,6 +11,18 @@ inputs: head-sha: description: "Head commit SHA" required: true + capture-command: + description: "Benchmark Capture command" + required: true + compare-command: + description: "Benchmark Comparison command" + required: true + base-results-path: + description: "Relative base results directory" + required: true + head-results-path: + description: "Relative head results directory" + required: true outputs: artifact-id: @@ -28,15 +40,12 @@ runs: path: benchmark-base persist-credentials: false - - name: "๐Ÿ“ฅ install base dependencies" + - name: "โš™๏ธ set up base" shell: bash working-directory: benchmark-base - run: pnpm install --frozen-lockfile - - - name: "๐Ÿ“ฆ build base" - shell: bash - working-directory: benchmark-base - run: pnpm build + run: | + pnpm install --frozen-lockfile + pnpm build - name: "๐Ÿ“‚ stage base output" shell: bash @@ -46,25 +55,31 @@ runs: - name: "๐Ÿ“Š capture base benchmark" shell: bash - run: pnpm bench:full --mode capture --reporter=default + env: + COMMAND: ${{ inputs.capture-command }} + run: bash -eo pipefail -c "$COMMAND" - - name: "๐Ÿ“ฆ build head" + - name: "โš™๏ธ set up head" shell: bash run: pnpm build - name: "๐Ÿ“Š compare benchmark" shell: bash - run: pnpm bench:full --mode compare --reporter=default --reporter=github-actions + env: + COMMAND: ${{ inputs.compare-command }} + run: bash -eo pipefail -c "$COMMAND" - name: "๐Ÿ“‚ collect results" shell: bash env: BASE_SHA: ${{ inputs.base-sha }} HEAD_SHA: ${{ inputs.head-sha }} + BASE_RESULTS_PATH: ${{ inputs.base-results-path }} + HEAD_RESULTS_PATH: ${{ inputs.head-results-path }} run: | - mkdir -p benchmark-results/full/current - cp .vitest/benchmarks/full/*.json benchmark-results/full/ - cp .vitest/benchmarks/full/current/*.json benchmark-results/full/current/ + mkdir -p benchmark-results/base benchmark-results/head + cp "$BASE_RESULTS_PATH"/*.json benchmark-results/base/ + cp "$HEAD_RESULTS_PATH"/*.json benchmark-results/head/ node -e 'require("node:fs").writeFileSync("benchmark-results/measurement.json", JSON.stringify({base: process.env.BASE_SHA, head: process.env.HEAD_SHA}))' - name: "๐Ÿ“ค upload results" diff --git a/.github/actions/benchmark-report/action.yml b/.github/actions/benchmark-report/action.yml index 77c13f4..b253a50 100644 --- a/.github/actions/benchmark-report/action.yml +++ b/.github/actions/benchmark-report/action.yml @@ -11,6 +11,12 @@ inputs: head-sha: description: "Head commit SHA" required: true + unit: + description: "Mean latency unit label" + default: "ms/operation" + operations-per-sample: + description: "Operations p/sample" + default: "1" runs: using: composite @@ -27,6 +33,8 @@ runs: BASE_SHA: ${{ inputs.base-sha }} HEAD_SHA: ${{ inputs.head-sha }} RESULTS: ${{ runner.temp }}/benchmark-results + RESULT_UNIT: ${{ inputs.unit }} + OPERATIONS_PER_SAMPLE: ${{ inputs.operations-per-sample }} with: script: | const { publish } = await import(`${process.env.GITHUB_WORKSPACE}/.github/scripts/benchmark-report.mjs`); diff --git a/.github/scripts/benchmark-report.mjs b/.github/scripts/benchmark-report.mjs index 2740ba0..ef113d7 100644 --- a/.github/scripts/benchmark-report.mjs +++ b/.github/scripts/benchmark-report.mjs @@ -5,6 +5,8 @@ import { readFile, readdir } from 'node:fs/promises'; import path from 'node:path'; export async function publish({ github, context, core }) { + const operationsPerSample = Number(process.env.OPERATIONS_PER_SAMPLE ?? '1'); + const unit = process.env.RESULT_UNIT ?? 'ms/operation'; const source = { pr: context.payload.pull_request.number, run: context.runId, @@ -23,6 +25,12 @@ export async function publish({ github, context, core }) { let rows; try { + if (!Number.isFinite(operationsPerSample) || operationsPerSample <= 0) { + skip('operations per sample must be finite and positive'); + + return; + } + const measurement = await readJson(path.join(process.env.RESULTS, 'measurement.json')); if (measurement?.base !== source.base || measurement?.head !== source.head) { @@ -31,11 +39,11 @@ export async function publish({ github, context, core }) { return; } - const baseDirectory = path.join(process.env.RESULTS, 'full'); - const headDirectory = path.join(baseDirectory, 'current'); + const baseDirectory = path.join(process.env.RESULTS, 'base'); + const headDirectory = path.join(process.env.RESULTS, 'head'); const baseEntries = await readdir(baseDirectory); const headEntries = await readdir(headDirectory); - const baseFiles = baseEntries.filter((file) => file !== 'current').toSorted((a, b) => a.localeCompare(b)); + const baseFiles = baseEntries.toSorted((a, b) => a.localeCompare(b)); const headFiles = headEntries.toSorted((a, b) => a.localeCompare(b)); if ( @@ -53,12 +61,12 @@ export async function publish({ github, context, core }) { for (const file of baseFiles) { const baseResult = await readJson(path.join(baseDirectory, file)); const headResult = await readJson(path.join(headDirectory, file)); - const base = Number.isFinite(baseResult?.latency?.mean) ? baseResult.latency.mean / 4 : Number.NaN; - const head = Number.isFinite(headResult?.latency?.mean) ? headResult.latency.mean / 4 : Number.NaN; + const base = Number.isFinite(baseResult?.latency?.mean) ? baseResult.latency.mean / operationsPerSample : Number.NaN; + const head = Number.isFinite(headResult?.latency?.mean) ? headResult.latency.mean / operationsPerSample : Number.NaN; const change = (head / base - 1) * 100; if (!Number.isFinite(base) || base <= 0 || !Number.isFinite(head) || head <= 0 || !Number.isFinite(change)) { - skip('latency means and per-search values must be finite and positive'); + skip('latency means and normalized values must be finite and positive'); return; } @@ -122,7 +130,7 @@ export async function publish({ github, context, core }) { ``, '## Benchmark Comparison', '', - '| Workload | Base, ms/search | PR, ms/search | Change |', + `| Workload | Base, ${unit} | PR, ${unit} | Change |`, '| --- | ---: | ---: | ---: |', ...rows, '', diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml index 9aec5c8..e65bed0 100644 --- a/.github/workflows/benchmark.yml +++ b/.github/workflows/benchmark.yml @@ -38,6 +38,10 @@ jobs: base-repository: ${{ github.event.pull_request.base.repo.full_name }} base-sha: ${{ github.event.pull_request.base.sha }} head-sha: ${{ github.event.pull_request.head.sha }} + capture-command: pnpm bench:full --mode capture --reporter=default + compare-command: pnpm bench:full --mode compare --reporter=default --reporter=github-actions + base-results-path: .vitest/benchmarks/full + head-results-path: .vitest/benchmarks/full/current report: name: "๐Ÿ“ report benchmarks" @@ -68,3 +72,5 @@ jobs: artifact-id: ${{ needs.benchmark.outputs.artifact-id }} base-sha: ${{ needs.benchmark.outputs.base }} head-sha: ${{ needs.benchmark.outputs.head }} + unit: ms/search + operations-per-sample: "4" diff --git a/docs/development.md b/docs/development.md index cd60524..808e606 100644 --- a/docs/development.md +++ b/docs/development.md @@ -102,7 +102,7 @@ Inspect manifest scripts/composition; keep related scripts/config/lock changes t [validate.yml](../.github/workflows/validate.yml) owns ordinary CI validation. CI runs without Nix/devenv. Local setup remains unchanged. -The PR-only [benchmark.yml](../.github/workflows/benchmark.yml) owns job permissions, head checkout, toolchain setup, job dependencies, producer outputs, and local action routing, separate from validation. The [capture composite action](../.github/actions/benchmark-capture/action.yml) owns base checkout, paired native execution, result collection, and upload. It captures the event base and compares the event head on one runner with the head harness. Each revision uses its own frozen lockfile under the head-selected toolchain. +The PR-only [benchmark.yml](../.github/workflows/benchmark.yml) owns job permissions, head checkout, toolchain setup, job dependencies, producer outputs, and local action routing, separate from validation. It also owns benchmark capture and comparison commands, result paths, and the paired head-harness configuration. The [capture composite action](../.github/actions/benchmark-capture/action.yml) owns standard pnpm installation, builds, and base output transfer into the head harness. It checks out the base into `benchmark-base`, executes the supplied benchmark commands, and uploads results. It collects immediate JSON files from the supplied paths into artifact directories `base` and `head`. The workflow captures the event base and compares the event head on one runner with the head harness. Each revision uses its own frozen lockfile under the head-selected toolchain. After successful comparison, the job uploads native reference/candidate JSON and measurement SHAs in an attempt-specific artifact with short retention. It keeps no measurement history. The **๐Ÿ“Š compare benchmarks** job output retains the native comparison table, failure annotations, and test summary. Capture uses the native default reporter. Comparison selects the native default and GitHub Actions reporters. @@ -110,9 +110,9 @@ The same workflow has a read-only benchmark job and a report job with comment-wr The report job requires successful benchmarks and non-empty artifact/base/head outputs. It downloads the exact producer artifact ID from the current run. Failed-report-only reruns skip the report if those outputs are missing. Output retention for that rerun mode is not guaranteed here. A full rerun produces a fresh artifact ID. -The publisher requires non-empty sets of native JSON files with equal counts and the same safe workload IDs. It requires finite positive per-search means and measurement SHAs that match producer outputs. It checks that the PR remains open with the measured base/head before publication. Invalid, missing, ambiguous, detected stale, or superseded results leave the old comment unchanged. GitHub comment writes have no atomic compare-and-swap, so a PR update can race the final check and write. +The publisher requires non-empty sets of native JSON files with equal counts and the same safe workload IDs. It requires finite positive normalized means and measurement SHAs that match producer outputs. It checks that the PR remains open with the measured base/head before publication. Invalid, missing, ambiguous, detected stale, or superseded results leave the old comment unchanged. GitHub comment writes have no atomic compare-and-swap, so a PR update can race the final check and write. -The publisher writes one table to its job summary and one bot-authored sticky comment. Rows use workload IDs, mean latency divided by four in ms/search, and signed `(head/base - 1) * 100` change. Positive change means slower. Results are advisory, not a performance gate or a statistical significance claim. The workflow, composite actions, and publisher own their executable contracts. Refresh this projection when those owners change. +The publisher writes one table to its job summary and one bot-authored sticky comment. Rows use workload IDs, normalized mean latency, and signed `(head/base - 1) * 100` change. The report action defaults to `operations-per-sample: "1"` and `unit: "ms/operation"`. The workflow sets `operations-per-sample: "4"` and `unit: ms/search` to divide native millisecond means by four. The divisor must be finite and positive. The unit is a display label, not a time conversion. Positive change means slower. Results are advisory, not a performance gate or a statistical significance claim. The workflow, composite actions, and publisher own their executable contracts. Refresh this projection when those owners change. Reserve `.scratch/` for local development, never CI. From 23af0388a1480ddc046c2e8a8be324fc8cfb3b3b Mon Sep 17 00:00:00 2001 From: devlsh Date: Thu, 8 Oct 2026 19:02:32 +0200 Subject: [PATCH 5/7] style: add spacing in benchmark report script --- .github/scripts/benchmark-report.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/scripts/benchmark-report.mjs b/.github/scripts/benchmark-report.mjs index ef113d7..8319072 100644 --- a/.github/scripts/benchmark-report.mjs +++ b/.github/scripts/benchmark-report.mjs @@ -7,6 +7,7 @@ import path from 'node:path'; export async function publish({ github, context, core }) { const operationsPerSample = Number(process.env.OPERATIONS_PER_SAMPLE ?? '1'); const unit = process.env.RESULT_UNIT ?? 'ms/operation'; + const source = { pr: context.payload.pull_request.number, run: context.runId, From 4ddadf3096edbf54ebdbbecbe10857ed658d73b9 Mon Sep 17 00:00:00 2001 From: devlsh Date: Thu, 8 Oct 2026 13:34:04 +0200 Subject: [PATCH 6/7] test: add coverage reporting --- CONTRIBUTING.md | 2 + package.json | 1 + pnpm-lock.yaml | 108 ++++++++++++++++++++++++++++++++++++++++++- tests/search.test.ts | 22 +++++++++ vitest.config.ts | 3 ++ 5 files changed, 134 insertions(+), 2 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 4efad97..e30f569 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -56,6 +56,8 @@ To fix lint and formatting findings, run `pnpm lint:fix`, then `pnpm fmt`. Inspe Run `pnpm test` for the Vitest suite. For behavior changes, add or update tests at the public consumer seam and describe what you verified; static checks alone do not prove runtime behavior. +Run `pnpm test:coverage` for the same suite with coverage reports. [vitest.config.ts](vitest.config.ts) limits coverage to `src/**/*.ts`, without test fixtures or generated output. Coverage shows code execution, not proof of correct behavior. `pnpm check` does not run coverage. + For demo changes, run these additional checks from the repository root: ```sh diff --git a/package.json b/package.json index 660803e..24ab437 100644 --- a/package.json +++ b/package.json @@ -68,6 +68,7 @@ "@stylistic/eslint-plugin": "5.10.0", "@tsconfig/node24": "24.0.5", "@types/node": "24.19.0", + "@vitest/coverage-v8": "5.0.3", "lefthook": "2.1.14", "oxfmt": "0.71.0", "oxlint": "1.86.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 6e12de4..7a7922f 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -222,6 +222,9 @@ importers: '@types/node': specifier: 24.19.0 version: 24.19.0 + '@vitest/coverage-v8': + specifier: 5.0.3 + version: 5.0.3(vitest@5.0.3) lefthook: specifier: 2.1.14 version: 2.1.14 @@ -242,7 +245,7 @@ importers: version: 7.0.2 vitest: specifier: 5.0.3 - version: 5.0.3(@types/node@24.19.0)(vite@8.3.2(@types/node@24.19.0)(esbuild@0.28.1)) + version: 5.0.3(@types/node@24.19.0)(@vitest/coverage-v8@5.0.3)(vite@8.3.2(@types/node@24.19.0)(esbuild@0.28.1)) demo: devDependencies: @@ -270,6 +273,27 @@ importers: packages: + '@babel/helper-string-parser@7.29.7': + resolution: {integrity: sha512-Pb5ijPrZ89GDH8223L4UP8i6QApWxs04RbPQJTeWDV0/keR2E36MeKnyr6LYmUUvqRRI+Iv87SuF1W6ErINzYw==} + engines: {node: '>=6.9.0'} + + '@babel/helper-validator-identifier@7.29.7': + resolution: {integrity: sha512-qehxGkRj55h/ff8EMaJ+cYhyaKlHIxqYDn682wQD7RNp9UujOQsHog2uS0r2vzr4pW+sXf90NeeayjcNaX3fFg==} + engines: {node: '>=6.9.0'} + + '@babel/parser@7.29.9': + resolution: {integrity: sha512-CjXrNHTnvqBVqHgdBysY3vk2T8tpJHb5/RMeHJBTyVa9xgugCB0CJTx/3oO8RV2QRQP391RWpB7D6hLjm8V9uA==} + engines: {node: '>=6.0.0'} + hasBin: true + + '@babel/types@7.29.8': + resolution: {integrity: sha512-Vj1jF3cPfxg7OAfoI7QnVKLoILlm2JF9pnVHrX8qx7AHMiYWT+NDAA7jChlNgRS4WTLc/fD1lXLmPixluj+3Gg==} + engines: {node: '>=6.9.0'} + + '@bcoe/v8-coverage@1.0.2': + resolution: {integrity: sha512-6zABk/ECA/QYSCQ1NGiVwwbQerUCZ+TQbp64Q3AgmfNvurHH0j8TtXa1qbShXA6qqkpAj4V5W8pP6mLe1mcMqA==} + engines: {node: '>=18'} + '@cacheable/memory@2.2.0': resolution: {integrity: sha512-CTLKqLItRCEixEAewD3/j9DB3/o96gpTPD4eJ1v+DGOlxZRZncRQkGYqqnAGCscYd6RNeXfGeiuCphsPtqyIfQ==} @@ -1281,6 +1305,23 @@ packages: cpu: [x64] os: [win32] + '@vitest/coverage-v8@5.0.3': + resolution: {integrity: sha512-+klsyz7BvT1vCU28Zkfzms1Ia78XD0V41U3FUtRaa3S+vOr/EXvK1O6BvVD7l1RzEjm6SONR2aybOvDDf9u0mQ==} + peerDependencies: + '@vitest/browser': 5.0.3 + vitest: 5.0.3 + peerDependenciesMeta: + '@vitest/browser': + optional: true + + '@vitest/istanbul-lib-coverage@1.0.2': + resolution: {integrity: sha512-9J/JMwOf9AoJhAywhrn7ScKTL38hsWQP/qPG60OtaAFcQ5OXPwKsxZFlbnuCKmZ61m8/lGgHYnFpdyQZUvG/iA==} + engines: {node: '>=22'} + + '@vitest/istanbul-lib-report@1.0.2': + resolution: {integrity: sha512-gUsfXZJbzPamoIY5TvHFiMMoXESBrUMo+xqaj+rYrWI69+EnvRlYBlP96ZnHPY4vX8kyUpgAnFUCg5wZG/HkDQ==} + engines: {node: '>=22'} + '@vitest/mocker@5.0.3': resolution: {integrity: sha512-T8sWAIbkSyAjkwTcaEc3Iu0o9A27X1/kdXrizhZkGuSKScRQtRzclfAMpOTcGdXCsqxeWlpGy3XjqaW8CpLORg==} peerDependencies: @@ -1454,6 +1495,9 @@ packages: resolution: {integrity: sha512-Izi8RQcffqCeNVgFigKli1ssklIbpHnCYc6AknXGYoB6grJqyeby7jv12JUQgmTAnIDnbck1uxksT4dzN3PWBA==} engines: {node: '>=12'} + ast-v8-to-istanbul@1.0.7: + resolution: {integrity: sha512-kFL68AG6ajd8fg248zwM9GQrUWEp79gsmjum34OEXjs4yHuUMZfYKwOLW9GMmB4oNvVrj+EAGxsP7ye2UR9UlA==} + balanced-match@4.0.4: resolution: {integrity: sha512-BLrgEcRTwX2o6gGxGOCNyMvGSp35YofuYzw9h1IMTRmKqttAZZVU67bdb9Pr2vUHA8+j3i2tJfjO6C6+4myGTA==} engines: {node: 18 || 20 || >=22} @@ -1683,6 +1727,9 @@ packages: js-binary-schema-parser@2.0.3: resolution: {integrity: sha512-xezGJmOb4lk/M1ZZLTR/jaBHQ4gG/lqQnJqdIv4721DMggsa1bDVlHXNeHYogaIEHD9vCRv0fcL4hMA+Coarkg==} + js-tokens@10.0.0: + resolution: {integrity: sha512-lM/UBzQmfJRo9ABXbPWemivdCW8V2G8FHaHdypQaIy523snUjog0W71ayWXTjiR+ixeMyVHN2XcpnTd/liPg/Q==} + json-schema-traverse@0.4.1: resolution: {integrity: sha512-xbbCH5dCYU5T8LcEhhuh7HJ88HXuW3qsI3Y0zOZFKfZEHcpWiHU/Jxzk629Brsab/mMiHQti9wMP+845RPe3Vg==} @@ -1838,6 +1885,9 @@ packages: magic-string@1.4.2: resolution: {integrity: sha512-vG+rjFRj1PqdIBozIxAGMjPlOhaVe+GXpbttY/iSK7rGcJRMlwNJO7dcUwmUqkymsFLJiNGI06t4D7Fr7yRC9g==} + magicast@0.5.5: + resolution: {integrity: sha512-UicdXN8zQ3JHlxVq+28afMXPr1z7WNY6+7EJnzTdQWkTAlMLF5fNCCKxJHBQwGaNGR11581EiQmQzx73+MvszA==} + miniflare@5.20261001.0-alpha: resolution: {integrity: sha512-GaimS5mSIOMyvd16ga+e1/QkI8cmp3z35QPHFex0DktqNQf2RVZQwMPQXdyoUreUiFP/0Gpe2MoQInTyMAD5xA==} engines: {node: '>=22.0.0'} @@ -2036,6 +2086,10 @@ packages: resolution: {integrity: sha512-jBrmx4lYmaC9k/mgPbylxs7kBUxHtD8256up+HjLaDFfXScKJQyil+SWXSvhAtT7XHo+yTVlpB81PGmHP8oLSQ==} engines: {node: ^20.0.0 || >=22.0.0} + tinyrainbow@3.2.0: + resolution: {integrity: sha512-LgO3D9yZJjApUiuUfl9iFAwrtaX4+lok3wJIqttGoKCHlWUqHqbQpnxCf82L8FjgKsh4iGo78hqJwgL8F6To2A==} + engines: {node: '>=14.0.0'} + tree-kill@1.2.2: resolution: {integrity: sha512-L0Orpi8qGpRG//Nd+H90vFB+3iHnue1zSSGmNOOCh1GLJ7rUKVwV2HvijphGQS2UmhUZewS9VgvxYIdgr+fG1A==} hasBin: true @@ -2252,6 +2306,21 @@ packages: snapshots: + '@babel/helper-string-parser@7.29.7': {} + + '@babel/helper-validator-identifier@7.29.7': {} + + '@babel/parser@7.29.9': + dependencies: + '@babel/types': 7.29.8 + + '@babel/types@7.29.8': + dependencies: + '@babel/helper-string-parser': 7.29.7 + '@babel/helper-validator-identifier': 7.29.7 + + '@bcoe/v8-coverage@1.0.2': {} + '@cacheable/memory@2.2.0': dependencies: '@cacheable/utils': 2.5.0 @@ -2854,6 +2923,24 @@ snapshots: '@typescript/typescript-win32-x64@7.0.2': optional: true + '@vitest/coverage-v8@5.0.3(vitest@5.0.3)': + dependencies: + '@bcoe/v8-coverage': 1.0.2 + '@vitest/istanbul-lib-coverage': 1.0.2 + '@vitest/istanbul-lib-report': 1.0.2 + ast-v8-to-istanbul: 1.0.7 + magicast: 0.5.5 + obug: 2.2.1 + std-env: 4.3.0 + tinyrainbow: 3.2.0 + vitest: 5.0.3(@types/node@24.19.0)(@vitest/coverage-v8@5.0.3)(vite@8.3.2(@types/node@24.19.0)(esbuild@0.28.1)) + + '@vitest/istanbul-lib-coverage@1.0.2': {} + + '@vitest/istanbul-lib-report@1.0.2': + dependencies: + '@vitest/istanbul-lib-coverage': 1.0.2 + '@vitest/mocker@5.0.3(vite@8.3.2(@types/node@24.19.0)(esbuild@0.28.1))': dependencies: '@jridgewell/trace-mapping': 0.3.31 @@ -2958,6 +3045,12 @@ snapshots: assertion-error@2.0.1: {} + ast-v8-to-istanbul@1.0.7: + dependencies: + '@jridgewell/trace-mapping': 0.3.31 + estree-walker: 3.0.3 + js-tokens: 10.0.0 + balanced-match@4.0.4: {} blake3-wasm@2.1.5: {} @@ -3189,6 +3282,8 @@ snapshots: js-binary-schema-parser@2.0.3: {} + js-tokens@10.0.0: {} + json-schema-traverse@0.4.1: {} json-stable-stringify-without-jsonify@1.0.1: {} @@ -3306,6 +3401,12 @@ snapshots: dependencies: '@jridgewell/sourcemap-codec': 1.6.0 + magicast@0.5.5: + dependencies: + '@babel/parser': 7.29.9 + '@babel/types': 7.29.8 + source-map-js: 1.2.2 + miniflare@5.20261001.0-alpha(@types/node@24.19.0): dependencies: '@cspotcode/source-map-support': 0.8.1 @@ -3546,6 +3647,8 @@ snapshots: tinypool@2.2.0: {} + tinyrainbow@3.2.0: {} + tree-kill@1.2.2: {} tsdown@0.23.0(typescript@7.0.2): @@ -3634,7 +3737,7 @@ snapshots: esbuild: 0.28.1 fsevents: 2.3.3 - vitest@5.0.3(@types/node@24.19.0)(vite@8.3.2(@types/node@24.19.0)(esbuild@0.28.1)): + vitest@5.0.3(@types/node@24.19.0)(@vitest/coverage-v8@5.0.3)(vite@8.3.2(@types/node@24.19.0)(esbuild@0.28.1)): dependencies: '@types/chai': 5.2.3 '@vitest/mocker': 5.0.3(vite@8.3.2(@types/node@24.19.0)(esbuild@0.28.1)) @@ -3652,6 +3755,7 @@ snapshots: why-is-node-running: 3.2.1 optionalDependencies: '@types/node': 24.19.0 + '@vitest/coverage-v8': 5.0.3(vitest@5.0.3) transitivePeerDependencies: - msw diff --git a/tests/search.test.ts b/tests/search.test.ts index d8d851b..0f886dc 100644 --- a/tests/search.test.ts +++ b/tests/search.test.ts @@ -49,6 +49,28 @@ describe('search', () => { ]); }); + test.concurrent('should use a shorter route discovered after a detour', () => { + const path = search({ + from: [0, 0], + to: [4, 0], + grid: [ + [0, 0, 0, 0, 0], + [0, 0, 0, -1, -1], + ], + // Zero estimates favor the lower detour before the direct route reaches [2, 0]. + // Both rows underestimate or equal the remaining distance to the goal. + heuristic: (current, goal) => (current[1] === 0 ? goal[0] - current[0] : 0), + }); + + expect(path).toStrictEqual([ + [0, 0], + [1, 0], + [2, 0], + [3, 0], + [4, 0], + ]); + }); + describe('search compatibility', () => { test.concurrent('retains frontier order on equal scores', () => { expect( diff --git a/vitest.config.ts b/vitest.config.ts index 36596ce..30cb795 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -6,5 +6,8 @@ export default defineConfig({ clearMocks: true, mockReset: true, restoreMocks: true, + coverage: { + include: ['src/**/*.ts'], + }, }, }); From 30446315cd6eb37db6e895b633fcf47285d68fc7 Mon Sep 17 00:00:00 2001 From: devlsh Date: Thu, 8 Oct 2026 18:18:36 +0200 Subject: [PATCH 7/7] docs: streamline contribution and development guidance --- AGENTS.md | 9 ++- CONTRIBUTING.md | 79 ++++++++++------------- demo/AGENTS.md | 6 +- docs/development.md | 148 +++++++------------------------------------- docs/releasing.md | 44 ++----------- 5 files changed, 68 insertions(+), 218 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index d397283..8958ade 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -16,10 +16,9 @@ Use `pnpm` for repository work, not `npm` or `yarn`. Executable files own discov Read the smallest applicable owner before editing, reviewing, or deeply analyzing its subject: -- **Contribute or validate** - Read [CONTRIBUTING.md](CONTRIBUTING.md) for shared human contribution and setup procedures, then [Agent Workflow](docs/development.md#agent-workflow) for agent authorization, tracker discipline, tooling, cumulative validation, and closeout. Skills naming `docs/agents/issue-tracker.md` or `docs/agents/triage-labels.md` route to [Tracker Operations](docs/development.md#tracker-operations); do not create duplicate compatibility files. -- **Develop the package** - Read [docs/development.md](docs/development.md) for responsibilities, public contracts, source authoring, TypeScript, and comments. -- **Change documentation or routing** - Read [Documentation Authority](docs/development.md#documentation-authority) before changing documentation, instructions, or their pointers. -- **Develop or validate the demo** - Read [demo/AGENTS.md](demo/AGENTS.md) for canvas ownership, lifecycle, deployment packaging, and workflow boundaries. Shared standards remain in [docs/development.md](docs/development.md). -- **Release or recover** - Read [docs/releasing.md](docs/releasing.md) for hosted readiness, authorization, prepare/publish, verification, and partial failures. +- **Contribute or validate** - Read [CONTRIBUTING.md](CONTRIBUTING.md) for setup, checks, and pull requests. Read [Agent Workflow](docs/development.md#agent-workflow) for consumer checks and results. Skills with `docs/agents/issue-tracker.md` or `docs/agents/triage-labels.md` routes use [Tracker Operations](docs/development.md#tracker-operations). Do not create duplicate compatibility files. +- **Develop the package or change documentation** - Read [docs/development.md](docs/development.md) for package development and consumer checks. For instructions or routing changes, read its [Documentation](docs/development.md#documentation) section. +- **Develop or validate the demo** - Read [demo/AGENTS.md](demo/AGENTS.md) for canvas ownership, lifecycle, deployment packaging, and behavior checks. Use [Agent Workflow](docs/development.md#agent-workflow) for shared checks and results. +- **Release or recover** - Read [docs/releasing.md](docs/releasing.md) for authorization, readiness, completion, and recovery. Update this file only for always-loaded authority, hard constraints, or task routing. Put branch-specific policy in its named owner and update affected links together. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index e30f569..0c0216d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -1,62 +1,56 @@ # Contributing -Read the [Code of Conduct](CODE_OF_CONDUCT.md) before participating. This guide covers reporting problems, setting up your checkout, and submitting changes. +Read the [Code of Conduct](CODE_OF_CONDUCT.md) before participating. ## Questions And Reports Use [GitHub Discussions](https://github.com/devlsh/astar/discussions) for questions and support, and [Issues](https://github.com/devlsh/astar/issues) for bugs and feature requests. Report vulnerabilities privately as described in [SECURITY.md](SECURITY.md). -Search open and closed issues before opening a new one. If you find a duplicate, add useful details there. Otherwise, describe the expected and actual behavior, steps to reproduce it, and relevant environment details. Distinguish what you observed from what you think caused it. +Search open and closed issues first. Add details to an existing report, or open a new one with reproduction steps, expected and actual behavior, and environment details. ## Local Development -### Requirements +Install [Nix](https://nix.dev/) and [devenv](https://devenv.sh/), then clone the repository. -- [Nix](https://nix.dev/) -- [devenv](https://devenv.sh/) -- [direnv](https://direnv.net/) _(Optional)_ +From the repository root, install dependencies with the frozen lockfile and set up Git hooks: -The Nix environment selects Node and pnpm major package families from locked inputs. It does not verify exact versions. [package.json](package.json) declares the exact required versions in `devEngines`; applicable pnpm commands reject mismatches. - -### Workflow - -1. Enter the cloned repo. If you're using `direnv`, allow the `.envrc` for the repository: +```sh +devenv tasks run astar:install +``` - ```sh - direnv allow . - ``` +Enter the development shell before running the pnpm commands below: - To revoke approval, run `direnv deny .` and leave the directory to unload its environment. For manual activation, omit or disable your host shell's direnv hook and use the explicit devenv commands below. Those commands alone do not disable an existing hook. +```sh +devenv shell +``` -2. Confirm the current [dependency policy](#dependency-changes), then install dependencies with the frozen lockfile. This also installs Git hooks: +If you use [direnv](https://direnv.net/), run `direnv allow .` instead of entering the shell manually. - ```sh - devenv tasks run astar:install - ``` +## Dependency Changes -3. Before changing source, read the relevant implementation, tests, public usage examples, and package exports. Run static checks before requesting review: +Ask the maintainer to approve dependency versions before adding or updating them. Include the manifest, lockfile, and any related script or configuration changes in the same PR. - ```sh - devenv --no-tui shell -- pnpm check - ``` +## Checks -With an activated environment, use `pnpm