feat(game): add nonogram as third daily puzzle type#1
Merged
Conversation
Introduce 8×8 picross with a 30-pattern library, daily selection, persistence, in-game grid UI, and result reveal. Fix column clue display order and add pattern regression tests for clue consistency across mirror transforms. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds a third puzzle game type, Nonogram (Picross), to the daily puzzle app. It includes type definitions, core grid algorithms, deterministic puzzle generation using 30 curated patterns, daily game integration, persistent storage support, interactive UI components, and comprehensive test coverage across all layers. ChangesNonogram Game Type
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
__tests__/lib/puzzles/dailySelector.test.ts (1)
60-68: ⚡ Quick winAdd puzzle-hash coherence assertion for nonogram, consistent with other game types.
This test should also verify that selector-level
puzzleHashmatches the puzzle payload hash.Proposed diff
it('returns real nonogram puzzle when type is nonogram', () => { const result = selectDailyGame({ dateKey: '2026-05-16', forceGameType: 'nonogram', }); expect(result.gameType).toBe('nonogram'); - expect(isNonogramPuzzle(result.puzzle)).toBe(true); + expect(isNonogramPuzzle(result.puzzle)).toBe(true); + if (isNonogramPuzzle(result.puzzle)) { + expect(result.puzzle.puzzleHash).toBe(result.puzzleHash); + } expect(result.puzzleHash).toMatch(/^nono-/); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/lib/puzzles/dailySelector.test.ts` around lines 60 - 68, The test for selectDailyGame should also assert that the selector-level puzzleHash equals the hash of the returned puzzle payload: recompute the puzzle hash from result.puzzle (using your project’s puzzle hashing utility, e.g. computePuzzleHash or similar) and add an assertion like expect(result.puzzleHash).toBe(computePuzzleHash(result.puzzle)); keep the existing checks (selectDailyGame, isNonogramPuzzle) and add this single coherence assertion so nonogram hashes are validated the same way as other game types.__tests__/lib/puzzles/nonogram/validate.test.ts (1)
30-33: ⚡ Quick winMake the negative-case assertion independent of a specific cell value.
This case currently assumes the first cell is empty. Force a mismatch from the solution so the test remains stable if pattern/seed internals change.
Proposed diff
it('is not complete when a fill cell is wrong', () => { - const play = createEmptyGrid(); - play[0]![0] = NONOGRAM_FILL; + const play = puzzle.solution.map((row) => + row.map((filled) => (filled ? NONOGRAM_FILL : -1)), + ); + play[0]![0] = puzzle.solution[0]![0] ? -1 : NONOGRAM_FILL; expect(isCompleteAndValid(play, puzzle.solution)).toBe(false); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/lib/puzzles/nonogram/validate.test.ts` around lines 30 - 33, Test currently assumes play[0][0] is empty; instead force a guaranteed mismatch with the solution by assigning play[0]![0] to a value different from puzzle.solution[0]![0] (for example, choose between NONOGRAM_FILL and NONOGRAM_EMPTY based on equality) before asserting isCompleteAndValid(play, puzzle.solution) is false; update the test that uses createEmptyGrid, NONOGRAM_FILL, and isCompleteAndValid to compute a value != puzzle.solution[0]![0] so the negative-case is stable regardless of pattern/seed internals.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/grid/NonogramGrid.tsx`:
- Around line 89-92: The computed cellSize can become zero or negative when
gridInner is non-positive; update the calculation in NonogramGrid.tsx so
cellSize is clamped to a minimum of 1 (e.g. compute rawSize =
Math.floor(Math.min(gridInner / cols, 36)) and then set cellSize = Math.max(1,
rawSize)) and keep clueColWidth and clueRowHeight computed from that clamped
cellSize (references: gridInner, cols, cellSize, clueColWidth, clueRowHeight,
maxWidth, clueBand).
In `@components/result/NonogramRevealCard.tsx`:
- Line 18: The computed cellSize in NonogramRevealCard (const cellSize =
Math.floor(size / Math.max(rows, cols))) can become 0 or negative for very small
size values; change the calculation to clamp the result to a minimum of 1 (e.g.,
wrap Math.floor(...) with Math.max(1, ...)) so cellSize is always positive and
cell rendering won't break.
In `@lib/puzzles/nonogram/generator.ts`:
- Line 23: Replace the non-null assertion on
NONOGRAM_PATTERNS[selectPatternIndex(seed)] with an explicit fail-fast guard:
first compute the index via selectPatternIndex(seed), then look up pattern =
NONOGRAM_PATTERNS[index]; if pattern is undefined, throw a clear Error that
includes context (e.g., "No nonogram pattern found", the index, and the seed) so
callers get a diagnostic instead of a silent runtime crash; otherwise continue
using pattern as before.
In `@lib/puzzles/nonogram/validate.ts`:
- Around line 9-25: The function isCompleteAndValid currently assumes playState
matches solution dimensions; add validation at the top to ensure playState is
defined, playState.length === solution.length (rows) and every playState[row] is
an array with length === cols (solution[0].length) before accessing cells; if
any check fails return false (or early-fail) so the non-null assertions around
playState[row]![col]! and comparisons with NONOGRAM_FILL are safe.
In `@lib/storage/snapshotValidate.ts`:
- Around line 3-8: The validation currently rejects the cross-mark cell by
treating any cell that is neither NONOGRAM_EMPTY nor NONOGRAM_FILL as invalid;
update the snapshot validation logic (the function that iterates the grid and
uses NONOGRAM_EMPTY and NONOGRAM_FILL) to treat the cross mark as an allowed
non-filled state: change checks that assert a cell must be exactly
NONOGRAM_EMPTY or NONOGRAM_FILL to instead only reject truly unknown/invalid
values and when counting filled cells only count cells equal to NONOGRAM_FILL
(so cross marks are allowed but not counted as filled). Apply the same change to
the other validation/repair check referenced at the end of the file (the
condition around line 136) so cross-marked cells are accepted across all
nonogram validation paths.
---
Nitpick comments:
In `@__tests__/lib/puzzles/dailySelector.test.ts`:
- Around line 60-68: The test for selectDailyGame should also assert that the
selector-level puzzleHash equals the hash of the returned puzzle payload:
recompute the puzzle hash from result.puzzle (using your project’s puzzle
hashing utility, e.g. computePuzzleHash or similar) and add an assertion like
expect(result.puzzleHash).toBe(computePuzzleHash(result.puzzle)); keep the
existing checks (selectDailyGame, isNonogramPuzzle) and add this single
coherence assertion so nonogram hashes are validated the same way as other game
types.
In `@__tests__/lib/puzzles/nonogram/validate.test.ts`:
- Around line 30-33: Test currently assumes play[0][0] is empty; instead force a
guaranteed mismatch with the solution by assigning play[0]![0] to a value
different from puzzle.solution[0]![0] (for example, choose between NONOGRAM_FILL
and NONOGRAM_EMPTY based on equality) before asserting isCompleteAndValid(play,
puzzle.solution) is false; update the test that uses createEmptyGrid,
NONOGRAM_FILL, and isCompleteAndValid to compute a value !=
puzzle.solution[0]![0] so the negative-case is stable regardless of pattern/seed
internals.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 94761d0a-091f-4b3e-896a-9c6d3f91e8d2
📒 Files selected for processing (30)
__tests__/components/grid/NonogramGrid.test.tsx__tests__/lib/puzzles/dailySelector.test.ts__tests__/lib/puzzles/nonogram/clues.test.ts__tests__/lib/puzzles/nonogram/generator.test.ts__tests__/lib/puzzles/nonogram/patterns.test.ts__tests__/lib/puzzles/nonogram/validate.test.tsapp/game.tsxapp/result.tsxcomponents/dev/DevToolsPanel.tsxcomponents/game/NonogramGameSection.tsxcomponents/grid/NonogramGrid.tsxcomponents/result/NonogramRevealCard.tsxconstants/dev.tscontexts/DailyGameContext.tsxhooks/useGameBoardSession.tshooks/useNonogramBoard.tslib/copy/gameRules.tslib/daily/dailyHydrate.tslib/puzzles/dailySelector.tslib/puzzles/nonogram/clues.tslib/puzzles/nonogram/generator.tslib/puzzles/nonogram/grid.tslib/puzzles/nonogram/hash.tslib/puzzles/nonogram/patterns.tslib/puzzles/nonogram/spec.tslib/puzzles/nonogram/transform.tslib/puzzles/nonogram/validate.tslib/puzzles/types.tslib/storage/snapshotPrep.tslib/storage/snapshotValidate.ts
Comment on lines
+89
to
+92
| const gridInner = maxWidth - clueBand * 14 - 8; | ||
| const cellSize = Math.floor(Math.min(gridInner / cols, 36)); | ||
| const clueColWidth = clueBand * Math.max(12, cellSize * 0.55); | ||
| const clueRowHeight = clueBand * Math.max(12, cellSize * 0.55); |
There was a problem hiding this comment.
Clamp computed cellSize to a positive minimum.
gridInner can become non-positive on narrow layouts, which can yield zero/negative cell dimensions and break rendering. Clamp cellSize to at least 1.
Proposed fix
- const cellSize = Math.floor(Math.min(gridInner / cols, 36));
+ const cellSize = Math.max(1, Math.floor(Math.min(gridInner / cols, 36)));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const gridInner = maxWidth - clueBand * 14 - 8; | |
| const cellSize = Math.floor(Math.min(gridInner / cols, 36)); | |
| const clueColWidth = clueBand * Math.max(12, cellSize * 0.55); | |
| const clueRowHeight = clueBand * Math.max(12, cellSize * 0.55); | |
| const gridInner = maxWidth - clueBand * 14 - 8; | |
| const cellSize = Math.max(1, Math.floor(Math.min(gridInner / cols, 36))); | |
| const clueColWidth = clueBand * Math.max(12, cellSize * 0.55); | |
| const clueRowHeight = clueBand * Math.max(12, cellSize * 0.55); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/grid/NonogramGrid.tsx` around lines 89 - 92, The computed cellSize
can become zero or negative when gridInner is non-positive; update the
calculation in NonogramGrid.tsx so cellSize is clamped to a minimum of 1 (e.g.
compute rawSize = Math.floor(Math.min(gridInner / cols, 36)) and then set
cellSize = Math.max(1, rawSize)) and keep clueColWidth and clueRowHeight
computed from that clamped cellSize (references: gridInner, cols, cellSize,
clueColWidth, clueRowHeight, maxWidth, clueBand).
| size = 160, | ||
| }: NonogramRevealCardProps) { | ||
| const { rows, cols, solution, pictureTitle } = puzzle; | ||
| const cellSize = Math.floor(size / Math.max(rows, cols)); |
There was a problem hiding this comment.
Ensure reveal cellSize stays positive.
A very small size prop can produce cellSize <= 0, which breaks cell rendering. Clamp to a minimum of 1.
Proposed fix
- const cellSize = Math.floor(size / Math.max(rows, cols));
+ const cellSize = Math.max(1, Math.floor(size / Math.max(rows, cols)));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const cellSize = Math.floor(size / Math.max(rows, cols)); | |
| const cellSize = Math.max(1, Math.floor(size / Math.max(rows, cols))); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/result/NonogramRevealCard.tsx` at line 18, The computed cellSize
in NonogramRevealCard (const cellSize = Math.floor(size / Math.max(rows, cols)))
can become 0 or negative for very small size values; change the calculation to
clamp the result to a minimum of 1 (e.g., wrap Math.floor(...) with Math.max(1,
...)) so cellSize is always positive and cell rendering won't break.
| } | ||
|
|
||
| export function generateNonogramPuzzle(seed: number): NonogramPuzzle { | ||
| const pattern = NONOGRAM_PATTERNS[selectPatternIndex(seed)]!; |
There was a problem hiding this comment.
Replace the non-null assertion with an explicit fail-fast guard.
Line 23 currently assumes a pattern always exists. If the catalog is ever empty/corrupted, this turns into a runtime crash path with weak diagnostics.
Suggested patch
function selectPatternIndex(seed: number): number {
+ if (NONOGRAM_PATTERNS.length === 0) {
+ throw new Error('NONOGRAM_PATTERNS must include at least one pattern');
+ }
const rng = mulberry32(deriveSubSeed(seed, 'nono-pattern'));
return Math.floor(rng() * NONOGRAM_PATTERNS.length);
}
export function generateNonogramPuzzle(seed: number): NonogramPuzzle {
- const pattern = NONOGRAM_PATTERNS[selectPatternIndex(seed)]!;
+ const pattern = NONOGRAM_PATTERNS[selectPatternIndex(seed)];
+ if (!pattern) {
+ throw new Error('Selected nonogram pattern is undefined');
+ }
const transform = pickTransform(seed);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/puzzles/nonogram/generator.ts` at line 23, Replace the non-null assertion
on NONOGRAM_PATTERNS[selectPatternIndex(seed)] with an explicit fail-fast guard:
first compute the index via selectPatternIndex(seed), then look up pattern =
NONOGRAM_PATTERNS[index]; if pattern is undefined, throw a clear Error that
includes context (e.g., "No nonogram pattern found", the index, and the seed) so
callers get a diagnostic instead of a silent runtime crash; otherwise continue
using pattern as before.
Comment on lines
+9
to
+25
| export function isCompleteAndValid( | ||
| playState: NonogramCell[][], | ||
| solution: boolean[][], | ||
| ): boolean { | ||
| const rows = solution.length; | ||
| const cols = solution[0]?.length ?? 0; | ||
|
|
||
| for (let row = 0; row < rows; row += 1) { | ||
| for (let col = 0; col < cols; col += 1) { | ||
| const shouldFill = solution[row]![col]!; | ||
| const cell = playState[row]![col]!; | ||
| if (shouldFill && cell !== NONOGRAM_FILL) return false; | ||
| if (!shouldFill && cell === NONOGRAM_FILL) return false; | ||
| } | ||
| } | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Add dimension validation before accessing cells.
The function assumes playState dimensions match solution dimensions but doesn't validate this. If dimensions mismatch, the non-null assertions on lines 18-19 could access undefined, leading to incorrect validation results.
🛡️ Proposed fix to add dimension check
export function isCompleteAndValid(
playState: NonogramCell[][],
solution: boolean[][],
): boolean {
const rows = solution.length;
const cols = solution[0]?.length ?? 0;
+
+ if (playState.length !== rows) return false;
+ if (playState[0]?.length !== cols) return false;
for (let row = 0; row < rows; row += 1) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/puzzles/nonogram/validate.ts` around lines 9 - 25, The function
isCompleteAndValid currently assumes playState matches solution dimensions; add
validation at the top to ensure playState is defined, playState.length ===
solution.length (rows) and every playState[row] is an array with length === cols
(solution[0].length) before accessing cells; if any check fails return false (or
early-fail) so the non-null assertions around playState[row]![col]! and
comparisons with NONOGRAM_FILL are safe.
Comment on lines
+3
to
+8
| import { | ||
| NONOGRAM_COLS, | ||
| NONOGRAM_EMPTY, | ||
| NONOGRAM_FILL, | ||
| NONOGRAM_ROWS, | ||
| } from '../puzzles/nonogram/spec'; |
There was a problem hiding this comment.
Allow cross-marked cells in nonogram play-state validation.
Line 40–43 currently rejects the cross state, so valid boards with crosses can be treated as inconsistent and get reset in repair paths.
💡 Proposed fix
import {
NONOGRAM_COLS,
+ NONOGRAM_CROSS,
NONOGRAM_EMPTY,
NONOGRAM_FILL,
NONOGRAM_ROWS,
} from '../puzzles/nonogram/spec';
@@
(cell) =>
cell === NONOGRAM_EMPTY ||
- cell === 0 ||
+ cell === NONOGRAM_CROSS ||
cell === NONOGRAM_FILL,
),
);
}Also applies to: 35-45, 136-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/storage/snapshotValidate.ts` around lines 3 - 8, The validation currently
rejects the cross-mark cell by treating any cell that is neither NONOGRAM_EMPTY
nor NONOGRAM_FILL as invalid; update the snapshot validation logic (the function
that iterates the grid and uses NONOGRAM_EMPTY and NONOGRAM_FILL) to treat the
cross mark as an allowed non-filled state: change checks that assert a cell must
be exactly NONOGRAM_EMPTY or NONOGRAM_FILL to instead only reject truly
unknown/invalid values and when counting filled cells only count cells equal to
NONOGRAM_FILL (so cross marks are allowed but not counted as filled). Apply the
same change to the other validation/repair check referenced at the end of the
file (the condition around line 136) so cross-marked cells are accepted across
all nonogram validation paths.
Regenerate package-lock.json with npm 10 so @emnapi/core and @emnapi/runtime are fully locked for wasm32-wasi transitive deps used by npm ci on Ubuntu. Co-authored-by: Cursor <cursoragent@cursor.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
1 participant
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.
Summary
reverse)patterns.test.ts(8×8, clue self-consistency, all mirror transforms),NonogramGrid.test.tsx, engine + selector coverageTest plan
npm run typechecknpm test(205 tests)npm run lintNotes
.planning/phases/nonogram/PLAN.mdupdated locally (gitignored)Made with Cursor
Summary by CodeRabbit
Release Notes
New Features
Tests