A bunch of upgrades by HappyPaul55 · Pull Request #3 · HappyPaul55/MeChessBook

HappyPaul55 · 2025-12-08T23:09:41Z

No description provided.

vercel · 2025-12-08T23:09:48Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Preview	Comments	Updated (UTC)
me-chess-book	Error			Dec 8, 2025 11:12pm

Copilot

Pull request overview

This PR implements several upgrades to the chess book generation system, including refactored game-to-page conversion logic, improved board rendering, enhanced move highlighting, and code organization improvements.

Extracted game pagination logic into a dedicated utility module with clustering-based game grouping
Refactored board rendering to compute board state during game processing rather than in components
Fixed move highlighting to correctly identify the position of interest in games

Reviewed changes

Copilot reviewed 15 out of 16 changed files in this pull request and generated 19 comments.

Show a summary per file

File	Description
utils/stringToNumber.ts	New utility for deterministic string-to-number conversion using hash-based approach
utils/convertGamesToPages.ts	Extracted and improved game pagination logic with length-based clustering and multi-page layout constraints
types.ts	Updated type definitions for ChessPage variants and Game structure to include board grid and move counts
package.json	Changed from private package to AGPL-3.0-only licensed
lichess.ts	Major refactoring: removed async, added analysis processing, moved board computation from components, switched from console.log to console.debug
components/Form/index.tsx	Removed await from now-synchronous processGame function call
components/Chess/MoveListTable.tsx	Fixed move highlighting to use correct ply calculation (removed -1 offset)
components/Chess/MoveListInline.tsx	Added move highlighting support with conditional span wrapping
components/Chess/ChessBoard.tsx	Simplified to render pre-computed board grid from game data, removing Chess.js dependency
components/Book/index.tsx	Refactored to use extracted utility functions, removed inline logic and debug console.log
components/Book/ThreeGamePage.tsx	Updated import path from lowercase 'page' to 'Page'
components/Book/SixGamePage.tsx	Enhanced layout with dynamic spacing based on move counts and conditional inline/table rendering
components/Book/Page.tsx	New file (renamed from page.tsx for proper casing)
components/Book/OneGamePage.tsx	Updated import path and removed unused Fragment import
components/Book/CoverPage.tsx	Updated import path from lowercase 'page' to 'Page'
components/Book/ContentPage.tsx	Minor formatting cleanup to consolidate template literal

Comments suppressed due to low confidence (2)

utils/convertGamesToPages.ts:40

Unused function distance.

function distance(a: Game, b: Game): number {

lichess.ts:176

Avoid automated semicolon insertion (92% of all statements in the enclosing function have an explicit semicolon).

    : randomNumberBetween(Math.max(0, Math.min(movesCount - 2, 3)), movesCount)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-08T23:15:58Z

lichess.ts

+  }
+  const plyOfInterest = biggestDeltaPly !== -1
+    ? biggestDeltaPly
+    : randomNumberBetween(Math.max(0, Math.min(movesCount - 2, 3)), movesCount)


Missing semicolon at the end of the statement. This should be : randomNumberBetween(Math.max(0, Math.min(movesCount - 2, 3)), movesCount); to maintain consistency with the rest of the codebase.

Suggested change

: randomNumberBetween(Math.max(0, Math.min(movesCount - 2, 3)), movesCount)

: randomNumberBetween(Math.max(0, Math.min(movesCount - 2, 3)), movesCount);

Copilot · 2025-12-08T23:15:59Z

utils/convertGamesToPages.ts

+/** Distance between two games for clustering (percentile based). */
+function distance(a: Game, b: Game): number {
+  const longer = Math.max(ply(a), ply(b));
+  const shorter = Math.min(ply(a), ply(b));
+  return (longer - shorter) / longer; // 0..1
+}
+


The distance function is defined but never used in this file. Consider removing it or documenting why it's kept for future use.

Suggested change

/** Distance between two games for clustering (percentile based). */

function distance(a: Game, b: Game): number {

const longer = Math.max(ply(a), ply(b));

const shorter = Math.min(ply(a), ply(b));

return (longer - shorter) / longer; // 0..1

}

Copilot · 2025-12-08T23:15:59Z

types.ts

+  games: [Game, Game, Game, Game],
+} | {
+  type: 'six',
+  games: [Game, Game, Game, Game, Game | undefined, Game | undefined],


The six page type allows 5th and 6th games to be undefined, but convertGamesToPages.ts line 140 casts a 3-game array to this type with the comment "6-page with only 3 games". This creates a type mismatch where positions 4-6 are undefined but the type only allows 5-6 to be undefined. The type should be [Game, Game, Game, Game | undefined, Game | undefined, Game | undefined] to accurately reflect the usage.

Suggested change

games: [Game, Game, Game, Game, Game | undefined, Game | undefined],

games: [Game, Game, Game, Game | undefined, Game | undefined, Game | undefined],

Copilot · 2025-12-08T23:15:59Z

lichess.ts

+  chess.loadPgn(game.pgn);
+  const turn = chess.turn();
+  while (chess.moveNumber() > 1 && (chess.moveNumber() - 1) * 2 + Number(turn === 'b') > plyOfInterest) {
+    chess.undo();
+  }
+


The Chess instance is created and the full PGN is loaded on line 178-179, then moves are undone one by one in a while loop (lines 181-183). For games with many moves, this is inefficient. Consider loading only the moves up to plyOfInterest instead of loading all moves and then undoing them.

Suggested change

chess.loadPgn(game.pgn);

const turn = chess.turn();

while (chess.moveNumber() > 1 && (chess.moveNumber() - 1) * 2 + Number(turn === 'b') > plyOfInterest) {

chess.undo();

}

// Play only up to plyOfInterest moves

for (let i = 0; i < plyOfInterest; i++) {

if (moves[i]) {

chess.move(moves[i], { sloppy: true });

}

}

const turn = chess.turn();

Copilot · 2025-12-08T23:16:00Z

components/Book/SixGamePage.tsx

    <ContentPage {...props}>
-      <div className="grid grid-cols-3 gap-4">
-        {props.games.map((game, i) => (
+      <div className={`grid grid-cols-3 gap-x-4 ${yGap} ${totalMoveRows}`}>


The class string includes ${totalMoveRows} which outputs a number directly into the className. This appears to be a debugging artifact and will create an invalid CSS class. Consider removing this debug value from the className.

Suggested change

<div className={`grid grid-cols-3 gap-x-4 ${yGap} ${totalMoveRows}`}>

<div className={`grid grid-cols-3 gap-x-4 ${yGap}`}>

Copilot · 2025-12-08T23:16:03Z

utils/convertGamesToPages.ts

+  const poolMedian = medianPly(pool);
+  const candidates = pool
+    .map((_, i) => pool.slice(i, i + 3))
+    .filter((g) => g.length === 3 && totalPly(g) <= 330)


The move limit check uses 330 half-moves (165 full moves), but the comment on line 60 says "165-move limit". This appears to be checking for 165 half-moves instead of 165 full moves. If the intention is to check for 165 full moves, the limit should be totalPly(g) <= 165 * 2 (330). If checking for 165 half-moves, update the comment. The current implementation is ambiguous.

Copilot · 2025-12-08T23:16:03Z

utils/convertGamesToPages.ts

+    .filter((g) => {
+      const top = maxPly(g.slice(0, 3));
+      const bot = maxPly(g.slice(3, 6));
+      return top + bot <= 200;


The 100-move row limit check uses 200 half-moves. The comment on line 76 says "100-move row limit", but this checks for 100 half-moves per row (200 total), not 100 full moves. If the intention is 100 full moves per row, the limit should be top + bot <= 100 * 2 * 2 (400). Please clarify the intended constraint and update either the code or comment accordingly.

Copilot · 2025-12-08T23:16:03Z

utils/convertGamesToPages.ts

+  }
+
+  /* 5. Done -------------------------------------------------------------- */
+  console.log({ pageCount: pages.length });


The console.log statement should either be removed for production code or changed to a proper logging mechanism. Debug logs like this can clutter production output.

Suggested change

console.log({ pageCount: pages.length });

Copilot · 2025-12-08T23:16:03Z

components/Book/SixGamePage.tsx

+
+  const totalMoveRows = sortedByMoveCountGames[0].movesCount + (sortedByMoveCountGames[3]?.movesCount ?? 0) / 2;
+
+  let yGap = 'gap-y-4';


The initial value of yGap is unused, since it is always overwritten.

Suggested change

let yGap = 'gap-y-4';

let yGap;

Copilot · 2025-12-08T23:16:03Z

lichess.ts

    // The game did not match the requirement of only showing wins or only losses.
-    console.log("filtering out " + game.id)
-    return;
+    console.debug("filtering out " + game.id)


Avoid automated semicolon insertion (92% of all statements in the enclosing function have an explicit semicolon).

Suggested change

console.debug("filtering out " + game.id)

console.debug("filtering out " + game.id);

A bunch of upgrades

1d2c48a

HappyPaul55 requested a review from Copilot December 8, 2025 23:09

Copilot started reviewing on behalf of HappyPaul55 December 8, 2025 23:10 View session

vercel bot had a problem deploying to Preview December 8, 2025 23:12 Failure

HappyPaul55 merged commit da1198c into main Dec 8, 2025
6 of 7 checks passed

Copilot AI reviewed Dec 8, 2025

View reviewed changes

	: randomNumberBetween(Math.max(0, Math.min(movesCount - 2, 3)), movesCount)
	: randomNumberBetween(Math.max(0, Math.min(movesCount - 2, 3)), movesCount);

	games: [Game, Game, Game, Game, Game \| undefined, Game \| undefined],
	games: [Game, Game, Game, Game \| undefined, Game \| undefined, Game \| undefined],

	<div className={`grid grid-cols-3 gap-x-4 ${yGap} ${totalMoveRows}`}>
	<div className={`grid grid-cols-3 gap-x-4 ${yGap}`}>


		const totalMoveRows = sortedByMoveCountGames[0].movesCount + (sortedByMoveCountGames[3]?.movesCount ?? 0) / 2;

		let yGap = 'gap-y-4';

	console.debug("filtering out " + game.id)
	console.debug("filtering out " + game.id);

Comments

Conversation

HappyPaul55 commented Dec 8, 2025

Uh oh!

vercel bot commented Dec 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

vercel bot commented Dec 8, 2025 •

edited

Loading