From a73b184beca684028c9cd2d75f8aa0e0278872d5 Mon Sep 17 00:00:00 2001 From: Mauricio Fernandes Date: Sat, 11 Jul 2026 22:04:26 -0300 Subject: [PATCH 1/2] fix(pathfinder): filter out-of-bounds row neighbors before indexing the matrix getValidNeighbors indexed `matrix[neighbor.row]![neighbor.col]` before any bounds check, so probing the NORTH neighbor of a start on row 0 (or the SOUTH neighbor on the last row) hit `matrix[-1]` (undefined) and threw a TypeError instead of being filtered out like out-of-bounds columns already were. Any pathfinding request whose start or goal sat on the grid's top/bottom edge crashed. Move the `isValidPosition` bounds check ahead of the matrix index (guarding row AND col) and drop the now-redundant trailing `isValid` term. Removes the stale `// TODO: handle grid edges`. Adds test/pathFinder.test.ts covering top-edge, bottom-edge, and every-neighbor-off-an-edge starts. Revert-dance verified: with the fix reverted all three tests fail with the TypeError at the old index line; restored, green. Co-Authored-By: Claude Opus 4.8 --- src/app/game/PathFinder.ts | 13 ++++-- test/pathFinder.test.ts | 88 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 4 deletions(-) create mode 100644 test/pathFinder.test.ts diff --git a/src/app/game/PathFinder.ts b/src/app/game/PathFinder.ts index 823c4bc..ccf91e4 100644 --- a/src/app/game/PathFinder.ts +++ b/src/app/game/PathFinder.ts @@ -101,19 +101,24 @@ export default class PathFinder { return false; } - // TODO: handle grid edges + // Bounds-check the row AND col before indexing the matrix. A neighbor off the top/bottom edge has + // no row entry (matrix[-1] is undefined), so indexing it before this guard threw a TypeError. + const isValid = this.field.isValidPosition(neighbor.row, neighbor.col); + if (!isValid || !destination) { + return false; + } + const neighborTile = matrix[neighbor.row]![neighbor.col]; - if (!neighborTile || !destination) { + if (!neighborTile) { return false; } - const isValid = this.field.isValidPosition(neighbor.row, neighbor.col); const isRoad = (neighborTile instanceof Road); // Every cell of the destination structure shares the same anchor identifier, so this lets A* step // through a building's footprint to reach its anchor cell from an adjacent road. const isDestination = (neighborTile.getIdentifier() === `${destination.row}-${destination.col}`); - return ( isValid && (isRoad || isDestination) ); + return (isRoad || isDestination); }); if (!validNeighbors) { diff --git a/test/pathFinder.test.ts b/test/pathFinder.test.ts new file mode 100644 index 0000000..16e3f17 --- /dev/null +++ b/test/pathFinder.test.ts @@ -0,0 +1,88 @@ +import Tile from '../src/app/game/Tile'; +import Road from '../src/app/game/Road'; +import PathFinder from '../src/app/game/PathFinder'; +import Field from '../src/app/game/Field'; + +import { TilePosition } from '../src/types/Position'; + +const FOOTPRINT_TILES = 3; + +// Mirrors the stamping helper in tileFootprint.test.ts: build a Field-like object whose matrix only +// holds the stamped structures, so an out-of-bounds row simply has no entry (matrix[-1] === undefined). +function stampField(structures: Tile[], rows: number, cols: number): Field { + const matrix: { [row: number]: { [col: number]: Tile } } = {}; + for (let row = 0; row < rows; row++) { + matrix[row] = {}; + } + + for (const structure of structures) { + for (const cell of structure.getFootprintCells(FOOTPRINT_TILES)) { + if (cell === null || cell.row < 0 || cell.row >= rows || cell.col < 0 || cell.col >= cols) { + continue; + } + matrix[cell.row]![cell.col] = structure; + } + } + + return { + matrix, + isValidPosition: (row: number, col: number) => row >= 0 && row < rows && col >= 0 && col < cols, + getTile: (row: number, col: number) => matrix[row]?.[col] ?? null, + } as unknown as Field; +} + +describe('PathFinder handles grid-edge starts and goals without crashing', () => { + test('a start on the grid top edge (row 0) finds a path instead of throwing', () => { + // Two adjacent road footprints on rows 0-2. The anchors sit on row 1, but the top footprint cells + // occupy row 0 — so expanding a node there probes a NORTH neighbor on row -1 (off the top edge). + const roadA = new Road(1, 1, null); // covers rows 0-2, cols 0-2 + const roadB = new Road(1, 4, null); // covers rows 0-2, cols 3-5 + + const field = stampField([roadA, roadB], 3, 6); + const pathFinder = new PathFinder(field); + + const start: TilePosition = { row: 0, col: 1 }; // top edge — NORTH neighbor is row -1 + const goal: TilePosition = { row: 0, col: 4 }; + + let path: Tile[] = []; + expect(() => { path = pathFinder.findPath(start, goal); }).not.toThrow(); + + // The off-top-edge neighbor is filtered out and A* still routes across the two footprints. + expect(path).toEqual([roadA, roadB]); + }); + + test('a start on the grid bottom edge (last row) finds a path instead of throwing', () => { + // Anchors on the last-but-one row so the footprint reaches the final grid row; the SOUTH neighbor + // of a node on that final row is off the bottom edge. + const rows = 3; + const roadA = new Road(1, 1, null); // covers rows 0-2, cols 0-2 (row 2 is the last row) + const roadB = new Road(1, 4, null); // covers rows 0-2, cols 3-5 + + const field = stampField([roadA, roadB], rows, 6); + const pathFinder = new PathFinder(field); + + const start: TilePosition = { row: rows - 1, col: 1 }; // bottom edge — SOUTH neighbor is row `rows` + const goal: TilePosition = { row: rows - 1, col: 4 }; + + let path: Tile[] = []; + expect(() => { path = pathFinder.findPath(start, goal); }).not.toThrow(); + + expect(path).toEqual([roadA, roadB]); + }); + + test('a single-footprint grid (every neighbor is off some edge) returns without crashing', () => { + // One road filling a 3x3 grid: from its anchor every N/S/E/W probe eventually lands off an edge. + const road = new Road(1, 1, null); // covers rows 0-2, cols 0-2 — the whole grid + const field = stampField([road], 3, 3); + const pathFinder = new PathFinder(field); + + const start: TilePosition = { row: 0, col: 0 }; // top-left corner: N and W neighbors are off-grid + const goal: TilePosition = { row: 2, col: 2 }; // same footprint anchor collapses to one step + + let path: Tile[] = []; + expect(() => { path = pathFinder.findPath(start, goal); }).not.toThrow(); + + // Start and goal share the road's footprint, so the collapsed path is the single road step. + expect(path).toEqual([road]); + }); +}); From ca6af1404da1198928a19270df1c4ffd9a7e3629 Mon Sep 17 00:00:00 2001 From: Mauricio Fernandes Date: Sat, 11 Jul 2026 23:03:21 -0300 Subject: [PATCH 2/2] test(agents): reconcile pathfinder edge tests with the game/test reorg merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Merging main (PR #93 game/test reorg) moved PathFinder to game/agents/ and brought in the canonical test/agents/pathFinder.test.ts, whose pinned test asserted the OLD crash ("a start on the grid top edge crashes..."). The grid-edge fix (carried onto agents/PathFinder.ts by rename detection) makes that no longer throw, so update the pinned test to assert the fixed behavior — no throw AND a valid path found — and add a matching bottom-edge case (both use unpadded matrices, mirroring a real Field's top/bottom edge). Remove the now-redundant flat test/pathFinder.test.ts (added before the reorg was on this branch); its cases are folded into the canonical agents test, and the flat path no longer matches any Jest project's testMatch. Revert-dance (agents module): with the pre-fix index-before-bounds-check restored, exactly the top- and bottom-edge tests fail with "Cannot read properties of undefined"; with the fix, all 97 agents tests pass. agents/PathFinder.ts coverage 89.6% stmts (module 93.3%), above the 80% gate. Co-Authored-By: Claude Opus 4.8 --- test/agents/pathFinder.test.ts | 42 ++++++++++++---- test/pathFinder.test.ts | 88 ---------------------------------- 2 files changed, 34 insertions(+), 96 deletions(-) delete mode 100644 test/pathFinder.test.ts diff --git a/test/agents/pathFinder.test.ts b/test/agents/pathFinder.test.ts index 3ae15c9..75fb3c8 100644 --- a/test/agents/pathFinder.test.ts +++ b/test/agents/pathFinder.test.ts @@ -192,13 +192,13 @@ describe('PathFinder A*', () => { expect(() => pathFinder.findPath({ row: 1, col: 0 }, { row: 1, col: 2 })).not.toThrow(); }); - // Documents a live edge-case limitation (acknowledged by the `// TODO: handle grid edges` comment in - // PathFinder.getValidNeighbors): a north/south neighbor lookup indexes `field.matrix[row]` BEFORE - // `isValidPosition` is consulted. A real Field's matrix only has entries for rows [0, rows), so probing - // the out-of-bounds north neighbor of a row-0 start (or the south neighbor of the last row) throws - // instead of being filtered out like an out-of-bounds column is. This pins the CURRENT behavior; it is - // not a desired outcome, and this test will need updating if the underlying bug is ever fixed. - test('a start on the grid top edge crashes probing the out-of-bounds north neighbor (documents a known edge-case bug)', () => { + // A north/south neighbor lookup used to index `field.matrix[row]` BEFORE `isValidPosition` was + // consulted. A real Field's matrix only has entries for rows [0, rows), so probing the out-of-bounds + // north neighbor of a row-0 start (or the south neighbor of the last row) threw a TypeError instead of + // being filtered out like an out-of-bounds column is. The fix bounds-checks the row before indexing; + // these two tests use UNPADDED matrices (only real rows exist, exactly like a real Field's top/bottom + // edge) to prove the edge is now filtered and the search still routes normally. + test('a start on the grid top edge filters the out-of-bounds north neighbor and still finds a path', () => { const road00 = new Road(0, 0, null); const road01 = new Road(0, 1, null); const road02 = new Road(0, 2, null); @@ -211,6 +211,32 @@ describe('PathFinder A*', () => { }; const pathFinder = new PathFinder(field as unknown as Field); - expect(() => pathFinder.findPath({ row: 0, col: 0 }, { row: 0, col: 2 })).toThrow(); + let path: Tile[] = []; + expect(() => { path = pathFinder.findPath({ row: 0, col: 0 }, { row: 0, col: 2 }); }).not.toThrow(); + expect(path).toEqual([road01, road02]); + }); + + test('a start on the grid bottom edge filters the out-of-bounds south neighbor and still finds a path', () => { + // A 3-row road grid where only rows 0..2 exist (no row 3), just like a real Field's bottom edge. + // Starting on the last row (row 2) probes a south neighbor on row 3, which must be filtered. + const roads: { [row: number]: { [col: number]: Road } } = {}; + for (let row = 0; row < 3; row++) { + roads[row] = {}; + for (let col = 0; col < 3; col++) { + roads[row]![col] = new Road(row, col, null); + } + } + const matrix = roads as unknown as { [row: number]: { [col: number]: Tile } }; + const field: FakeField = { + matrix: matrix as unknown as Field['matrix'], + isValidPosition: (row: number, col: number) => row >= 0 && row < 3 && col >= 0 && col < 3, + getTile: (row: number, col: number) => matrix[row]?.[col] ?? null, + }; + const pathFinder = new PathFinder(field as unknown as Field); + + let path: Tile[] = []; + expect(() => { path = pathFinder.findPath({ row: 2, col: 0 }, { row: 2, col: 2 }); }).not.toThrow(); + // Straight along the bottom row: (2,0) -> (2,1) -> (2,2). + expect(path).toEqual([roads[2]![1], roads[2]![2]]); }); }); diff --git a/test/pathFinder.test.ts b/test/pathFinder.test.ts deleted file mode 100644 index 16e3f17..0000000 --- a/test/pathFinder.test.ts +++ /dev/null @@ -1,88 +0,0 @@ -import Tile from '../src/app/game/Tile'; -import Road from '../src/app/game/Road'; -import PathFinder from '../src/app/game/PathFinder'; -import Field from '../src/app/game/Field'; - -import { TilePosition } from '../src/types/Position'; - -const FOOTPRINT_TILES = 3; - -// Mirrors the stamping helper in tileFootprint.test.ts: build a Field-like object whose matrix only -// holds the stamped structures, so an out-of-bounds row simply has no entry (matrix[-1] === undefined). -function stampField(structures: Tile[], rows: number, cols: number): Field { - const matrix: { [row: number]: { [col: number]: Tile } } = {}; - for (let row = 0; row < rows; row++) { - matrix[row] = {}; - } - - for (const structure of structures) { - for (const cell of structure.getFootprintCells(FOOTPRINT_TILES)) { - if (cell === null || cell.row < 0 || cell.row >= rows || cell.col < 0 || cell.col >= cols) { - continue; - } - matrix[cell.row]![cell.col] = structure; - } - } - - return { - matrix, - isValidPosition: (row: number, col: number) => row >= 0 && row < rows && col >= 0 && col < cols, - getTile: (row: number, col: number) => matrix[row]?.[col] ?? null, - } as unknown as Field; -} - -describe('PathFinder handles grid-edge starts and goals without crashing', () => { - test('a start on the grid top edge (row 0) finds a path instead of throwing', () => { - // Two adjacent road footprints on rows 0-2. The anchors sit on row 1, but the top footprint cells - // occupy row 0 — so expanding a node there probes a NORTH neighbor on row -1 (off the top edge). - const roadA = new Road(1, 1, null); // covers rows 0-2, cols 0-2 - const roadB = new Road(1, 4, null); // covers rows 0-2, cols 3-5 - - const field = stampField([roadA, roadB], 3, 6); - const pathFinder = new PathFinder(field); - - const start: TilePosition = { row: 0, col: 1 }; // top edge — NORTH neighbor is row -1 - const goal: TilePosition = { row: 0, col: 4 }; - - let path: Tile[] = []; - expect(() => { path = pathFinder.findPath(start, goal); }).not.toThrow(); - - // The off-top-edge neighbor is filtered out and A* still routes across the two footprints. - expect(path).toEqual([roadA, roadB]); - }); - - test('a start on the grid bottom edge (last row) finds a path instead of throwing', () => { - // Anchors on the last-but-one row so the footprint reaches the final grid row; the SOUTH neighbor - // of a node on that final row is off the bottom edge. - const rows = 3; - const roadA = new Road(1, 1, null); // covers rows 0-2, cols 0-2 (row 2 is the last row) - const roadB = new Road(1, 4, null); // covers rows 0-2, cols 3-5 - - const field = stampField([roadA, roadB], rows, 6); - const pathFinder = new PathFinder(field); - - const start: TilePosition = { row: rows - 1, col: 1 }; // bottom edge — SOUTH neighbor is row `rows` - const goal: TilePosition = { row: rows - 1, col: 4 }; - - let path: Tile[] = []; - expect(() => { path = pathFinder.findPath(start, goal); }).not.toThrow(); - - expect(path).toEqual([roadA, roadB]); - }); - - test('a single-footprint grid (every neighbor is off some edge) returns without crashing', () => { - // One road filling a 3x3 grid: from its anchor every N/S/E/W probe eventually lands off an edge. - const road = new Road(1, 1, null); // covers rows 0-2, cols 0-2 — the whole grid - const field = stampField([road], 3, 3); - const pathFinder = new PathFinder(field); - - const start: TilePosition = { row: 0, col: 0 }; // top-left corner: N and W neighbors are off-grid - const goal: TilePosition = { row: 2, col: 2 }; // same footprint anchor collapses to one step - - let path: Tile[] = []; - expect(() => { path = pathFinder.findPath(start, goal); }).not.toThrow(); - - // Start and goal share the road's footprint, so the collapsed path is the single road step. - expect(path).toEqual([road]); - }); -});