fix(pathfinder): crash on grid top/bottom-edge starts and goals#94
Merged
Conversation
…he 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 <noreply@anthropic.com>
…g merge 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 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
PathFinder.getValidNeighbors(src/app/game/PathFinder.ts) indexed the tilematrix before any bounds check:
Field.matrixis a{ [row]: { [col]: Tile } }map, so for a neighbor off thetop edge (
row === -1) or bottom edge (row === rows),matrix[row]isundefinedand indexing it throwsTypeError: Cannot read properties of undefined (reading '<col>'). Out-of-bounds columns were already handledgracefully (the col index just returns
undefined→ filtered), but rowscrashed. Any pathfinding request whose start or goal sits on the grid's
top/bottom edge (a very reachable state — roads/buildings can be placed against
the map border) crashed the pather.
The fix
Move the
field.isValidPosition(row, col)bounds check ahead of the matrixindex so an out-of-bounds row (or col) is filtered before indexing — exactly how
out-of-bounds columns were already tolerated. The trailing
isValidterm in thereturn is now guaranteed true, so it's dropped, and the stale
// TODO: handle grid edgesis removed.Tests
New
test/pathFinder.test.ts(there was no pre-existing pathfinder test on thisbranch) exercises real behavior — a valid collapsed path is returned, not merely
"doesn't throw":
Revert-dance (proves the test guards the fix)
npx jest test/pathFinder.test.tsgreen (3/3).git stash push -- src/app/game/PathFinder.ts(buggy source restored) → all 3 fail withTypeError: Cannot read properties of undefined (reading '0')at the oldmatrix[neighbor.row]!index line.git stash pop(fix restored) → green again.Observed the fail→pass transition, so the test genuinely catches the regression.
Gates
npx tsc --noEmit -p tsconfig.json— clean (exit 0).npx jest(full suite) — 73 suites / 647 tests green.src/app/game/PathFinder.ts— 85.71% stmts / 75% branch / 100% funcs (≥ 80% stmt bar).Notes on task vs. repo state
The task described a
src/app/game/agents/+test/agents/layout with--selectProjects agents, a per-module coverage project, an eslint gate, and apre-written pinned test. None of that exists on this branch:
PathFinder.tsisflat at
src/app/game/, tests are a single flattest/suite, there is noeslint config/dependency in the repo, and no pathfinder test existed. I adapted
to the actual repo — fixed the real bug at the real path, added a new flat test,
and ran the gates that exist here (tsc / jest / coverage). Test imports use
../src/...to match all 73 existing test files (the repo convention; no testuses path aliases).
🤖 Generated with Claude Code