Update GameContextImport trait to return Either[String, GameContext] instead of Option[GameContext] to provide better error context during parsing. Update FenParser implementation to convert Option to Either using toRight(). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
24 KiB
IO Interface Refactor Implementation Plan
For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (
- [ ]) syntax for tracking.
Goal: Unify IO import/export behind uniform interfaces so GameEngine accepts any format without format-specific imports.
Architecture: Change GameContextImport from Option to Either return; implement both FEN and PGN to this interface; refactor GameEngine to accept importer/exporter traits instead of hardcoded PgnParser.
Tech Stack: Scala 3, Either, GameContext, Quarkus, ScalaTest
File Structure
| File | Action | Responsibility |
|---|---|---|
modules/io/src/main/scala/de/nowchess/io/GameContextImport.scala |
Modify | Change signature from Option to Either[String, GameContext] |
modules/io/src/main/scala/de/nowchess/io/fen/FenParser.scala |
Modify | Implement GameContextImport, wrap parseFen with error messages |
modules/io/src/main/scala/de/nowchess/io/pgn/PgnParser.scala |
Modify | Implement GameContextImport, call validatePgn, build final GameContext with moves |
modules/io/src/main/scala/de/nowchess/io/pgn/PgnExporter.scala |
Modify | Implement GameContextExport, build PGN from context.moves with default headers |
modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala |
Modify | Add loadGame(importer, input), exportGame(exporter); remove loadPgn |
modules/io/src/test/scala/de/nowchess/io/fen/FenParserTest.scala |
Modify | Update assertions from Option to Either |
modules/io/src/test/scala/de/nowchess/io/pgn/PgnParserTest.scala |
Modify | Add importGameContext test cases |
modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala |
Modify | Add exportGameContext test cases |
modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineLoadPgnTest.scala |
Modify | Replace loadPgn calls with loadGame(PgnParser, …); add FEN load tests |
Task 1: Update GameContextImport interface
Files:
-
Modify:
modules/io/src/main/scala/de/nowchess/io/GameContextImport.scala -
Step 1: Read current GameContextImport
Current:
trait GameContextImport {
def importGameContext(input: String): Option[GameContext]
}
- Step 2: Change signature to Either
package de.nowchess.io
import de.nowchess.api.game.GameContext
trait GameContextImport:
def importGameContext(input: String): Either[String, GameContext]
- Step 3: Verify GameContextExport unchanged
Confirm it still exists as:
trait GameContextExport:
def exportGameContext(context: GameContext): String
Task 2: Update FenParser to implement Either
Files:
-
Modify:
modules/io/src/main/scala/de/nowchess/io/fen/FenParser.scala -
Step 1: Update import statements
Add imports at top:
package de.nowchess.io.fen
import de.nowchess.api.board.*
import de.nowchess.api.game.GameContext
import de.nowchess.io.GameContextImport
- Step 2: Update class signature
Change:
object FenParser extends GameContextImport:
(It already extends GameContextImport; verify it does)
- Step 3: Update parseFen to return Either and call importGameContext
Replace the current parseFen return logic. Keep the body as-is but wrap returns:
def parseFen(fen: String): Either[String, GameContext] =
val parts = fen.trim.split("\\s+")
if parts.length != 6 then
Left("Invalid FEN: expected 6 space-separated fields, got " + parts.length)
else
(for
board <- parseBoard(parts(0)).toRight("Invalid FEN: invalid board position")
activeColor <- parseColor(parts(1)).toRight("Invalid FEN: invalid active color (expected 'w' or 'b')")
castlingRights <- parseCastling(parts(2)).toRight("Invalid FEN: invalid castling rights")
enPassant <- parseEnPassant(parts(3)).toRight("Invalid FEN: invalid en passant square")
halfMoveClock <- parts(4).toIntOption.toRight("Invalid FEN: invalid half-move clock (expected integer)")
fullMoveNumber <- parts(5).toIntOption.toRight("Invalid FEN: invalid full move number (expected integer)")
if halfMoveClock >= 0 && fullMoveNumber >= 1
yield GameContext(
board = board,
turn = activeColor,
castlingRights = castlingRights,
enPassantSquare = enPassant,
halfMoveClock = halfMoveClock,
moves = List.empty
)).left.map(err => "Invalid FEN: " + err)
- Step 4: Implement importGameContext
def importGameContext(input: String): Either[String, GameContext] = parseFen(input)
- Step 5: Verify parseBoard, parseColor, parseCastling, parseEnPassant still return Option
They do. They stay as-is.
Task 3: Update FenParserTest for Either
Files:
-
Modify:
modules/io/src/test/scala/de/nowchess/io/fen/FenParserTest.scala -
Step 1: Update test "parse full FEN - initial position"
Change from:
context.isDefined shouldBe true
context.get.turn shouldBe Color.White
To:
context.isRight shouldBe true
context.getOrElse(??).turn shouldBe Color.White
Or use pattern match:
context match
case Right(ctx) =>
ctx.turn shouldBe Color.White
ctx.castlingRights.whiteKingSide shouldBe true
case Left(err) => fail(s"Expected Right but got Left: $err")
- Step 2: Update all "context.isDefined" to "context.isRight"
Search and replace: context.isDefined → context.isRight; context.get → context.getOrElse(???) or use pattern matching
- Step 3: Update error test cases
Change from:
context.isDefined shouldBe false
To:
context.isLeft shouldBe true
Example fixes:
-
Line 89:
context.isDefined shouldBe false→context.isLeft shouldBe true -
Line 95:
context.isDefined shouldBe false→context.isLeft shouldBe true -
Line 101:
context.isDefined shouldBe false→context.isLeft shouldBe true -
Step 4: Run FenParserTest
./gradlew :modules:io:test --tests "de.nowchess.io.fen.FenParserTest" -v
Expected: All tests pass.
Task 4: Implement PgnParser.importGameContext
Files:
-
Modify:
modules/io/src/main/scala/de/nowchess/io/pgn/PgnParser.scala -
Step 1: Add GameContextImport trait to object
Change:
object PgnParser:
To:
object PgnParser extends GameContextImport:
- Step 2: Implement importGameContext
Add this method to PgnParser:
def importGameContext(input: String): Either[String, GameContext] =
validatePgn(input).flatMap { game =>
// Replay moves to populate GameContext.moves via DefaultRules.applyMove
val (finalCtx, errors) = game.moves.foldLeft((GameContext.initial, Option.empty[String])) {
case ((ctx, Some(err)), _) => (ctx, Some(err)) // Already failed, stop
case ((ctx, None), histMove) =>
val moveOpt = parseAlgebraicMove(
s"${histMove.from}${histMove.to}",
ctx,
ctx.turn
)
moveOpt match
case None => (ctx, Some(s"Failed to parse move ${histMove.from}${histMove.to}"))
case Some(move) =>
val nextCtx = DefaultRules.applyMove(ctx, move)
(nextCtx, None)
}
errors match
case Some(err) => Left(err)
case None =>
if finalCtx.moves.isEmpty && game.moves.nonEmpty then
Left("No moves were parsed from the PGN")
else
Right(finalCtx)
}
- Step 3: Ensure imports include DefaultRules
At top of file:
import de.nowchess.rules.sets.DefaultRules
import de.nowchess.io.GameContextImport
- Step 4: Run PgnParserTest
./gradlew :modules:io:test --tests "de.nowchess.io.pgn.PgnParserTest" -v
Expected: All existing tests still pass (validatePgn is unchanged).
Task 5: Add importGameContext tests to PgnParserTest
Files:
-
Modify:
modules/io/src/test/scala/de/nowchess/io/pgn/PgnParserTest.scala -
Step 1: Add test for importGameContext with valid game
Append to PgnParserTest:
test("importGameContext: valid PGN returns Right with GameContext") {
val pgn = """[Event "Test"]
[White "A"]
[Black "B"]
1. e4 e5 2. Nf3 Nc6
"""
val result = PgnParser.importGameContext(pgn)
result.isRight shouldBe true
val ctx = result.getOrElse(???)
ctx.moves.length shouldBe 4
ctx.turn shouldBe Color.Black
}
- Step 2: Add test for importGameContext with invalid PGN
test("importGameContext: invalid PGN returns Left") {
val pgn = "[Event \"T\"]\n\n1. Qd4"
val result = PgnParser.importGameContext(pgn)
result.isLeft shouldBe true
result.left.getOrElse("").nonEmpty shouldBe true
}
- Step 3: Add test for empty moves
test("importGameContext: PGN with no moves returns Right with initial position") {
val pgn = "[Event \"T\"]\n[White \"A\"]\n[Black \"B\"]\n"
val result = PgnParser.importGameContext(pgn)
result.isRight shouldBe true
val ctx = result.getOrElse(???)
ctx.moves.length shouldBe 0
ctx.board shouldBe Board.initial
}
- Step 4: Run tests
./gradlew :modules:io:test --tests "de.nowchess.io.pgn.PgnParserTest" -v
Expected: All tests pass.
Task 6: Update PgnExporter to implement GameContextExport
Files:
-
Modify:
modules/io/src/main/scala/de/nowchess/io/pgn/PgnExporter.scala -
Step 1: Add trait to object signature
Change:
object PgnExporter:
To:
object PgnExporter extends GameContextExport:
- Step 2: Add GameContextExport import
At top:
import de.nowchess.io.GameContextExport
- Step 3: Refactor exportGame to use context.moves
Replace current exportGame implementation with one that builds PGN from GameContext.moves. The moves are List[Move] not List[HistoryMove], so convert:
def exportGame(context: GameContext): String =
// Build default headers if not present
val headers = Map(
"Event" -> "?",
"White" -> "?",
"Black" -> "?",
"Result" -> "*"
)
val headerLines = headers.map { case (k, v) =>
s"""[$k "$v"]"""
}.mkString("\n")
val moveText = if context.moves.isEmpty then ""
else
val grouped = context.moves.zipWithIndex.groupBy(_._2 / 2)
val lines = for (idx, movePairs) <- grouped.toList.sortBy(_._1) yield
val moveNum = idx + 1
val whiteStr = movePairs.find(_._2 % 2 == 0).map(p => moveToAlgebraicFromContext(p._1, context)).getOrElse("")
val blackStr = movePairs.find(_._2 % 2 == 1).map(p => moveToAlgebraicFromContext(p._1, context)).getOrElse("")
if blackStr.isEmpty then s"$moveNum. $whiteStr"
else s"$moveNum. $whiteStr $blackStr"
lines.mkString(" ") + " *"
if headerLines.isEmpty then moveText
else if moveText.isEmpty then headerLines
else s"$headerLines\n\n$moveText"
Wait, this is getting complex because context.moves is List[Move] but moveToAlgebraicFromContext needs the board state before the move. Let me revise:
def exportGameContext(context: GameContext): String =
// Use the existing GameHistory-based export for now, or
// If context.moves is empty, return headers only
if context.moves.isEmpty then
val headers = Map("Event" -> "?", "White" -> "?", "Black" -> "?")
headers.map { case (k, v) => s"""[$k "$v"]""" }.mkString("\n")
else
// Replay the game to track board state and generate notation
val headerLines = "".trim // No headers from context for now (TBD: store headers in GameContext)
val moveText = replayAndExport(context.moves)
if moveText.isEmpty then "" else moveText + " *"
private def replayAndExport(moves: List[Move]): String =
// This requires replaying moves to get board state before each move
// For now, a simplified version:
moves.zipWithIndex.map { case (move, idx) =>
val moveNum = idx / 2 + 1
val moveStr = move.moveType match
case MoveType.CastleKingside => "O-O"
case MoveType.CastleQueenside => "O-O-O"
case _ => s"${move.to}" // Simplified, loses disambiguation
val prefix = if idx % 2 == 0 then s"$moveNum. " else ""
prefix + moveStr
}.mkString(" ")
Actually, this is too complex. Let me keep the existing signature that takes headers separately for now, and just ensure exportGameContext delegates:
- Step 1 (revised): Implement exportGameContext
def exportGameContext(context: GameContext): String =
// Extract default headers and export from context.moves
val defaultHeaders = Map(
"Event" -> "?",
"White" -> "?",
"Black" -> "?",
"Result" -> "*"
)
exportGameWithHeaders(defaultHeaders, context)
private def exportGameWithHeaders(headers: Map[String, String], context: GameContext): String =
val headerLines = headers.map { case (key, value) =>
s"""[$key "$value"]"""
}.mkString("\n")
val moveText = if context.moves.isEmpty then ""
else
val groupedMoves = context.moves.zipWithIndex.groupBy(_._2 / 2)
val moveLines = for (moveNumber, movePairs) <- groupedMoves.toList.sortBy(_._1) yield
val moveNum = moveNumber + 1
val whiteMoveStr = movePairs.find(_._2 % 2 == 0).map(p => moveToAlgebraicFromMove(p._1)).getOrElse("")
val blackMoveStr = movePairs.find(_._2 % 2 == 1).map(p => moveToAlgebraicFromMove(p._1)).getOrElse("")
if blackMoveStr.isEmpty then s"$moveNum. $whiteMoveStr"
else s"$moveNum. $whiteMoveStr $blackMoveStr"
val termination = headers.getOrElse("Result", "*")
moveLines.mkString(" ") + s" $termination"
if headerLines.isEmpty then moveText
else if moveText.isEmpty then headerLines
else s"$headerLines\n\n$moveText"
private def moveToAlgebraicFromMove(move: Move): String =
move.moveType match
case MoveType.CastleKingside => "O-O"
case MoveType.CastleQueenside => "O-O-O"
case _ => move.to.toString // Simplified, loses piece disambiguation
Actually this is still lossy. The real solution requires replaying to get board state. For now, keep exportGame as-is (it takes GameHistory) and make exportGameContext a thin wrapper:
- Step 1 (final revision): Add exportGameContext that converts context to GameHistory
def exportGameContext(context: GameContext): String =
val headers = Map("Event" -> "?", "White" -> "?", "Black" -> "?", "Result" -> "*")
val history = GameHistory(
moves = context.moves.map(m =>
HistoryMove(m.from, m.to) // Simplified; loses castling/promotion info
),
halfMoveClock = context.halfMoveClock
)
exportGame(headers, history)
Wait, this loses information from context.moves (they're Move not HistoryMove).
Let me think: the spec says "PgnExporter generates PGN from ctx.moves with default headers". But ctx.moves is List[Move] which has no castling/promotion info baked in the structure — that's in moveType. We'd need to replay or store richer info.
For MVP: keep current exportGame(headers, history) working, and just ensure PgnExporter extends GameContextExport. We'll leave exportGameContext as a TODO or a simplified version that requires the caller to still use the old API for full fidelity.
- Step 1 (pragmatic): Add minimal exportGameContext
def exportGameContext(context: GameContext): String =
// Minimal implementation: returns empty PGN if moves present but no headers
if context.moves.isEmpty then ""
else
// For now, caller must use exportGame(headers, history) for full control
// This is a placeholder that returns a minimal PGN
"* (export via exportGame for full control)"
Actually, looking back at the spec: "generates PGN from ctx.moves with default headers". The problem is ctx.moves are Move, not HistoryMove. The conversion is lossy without a board replay.
Let me re-read the design... it says PgnExporter.exportGameContext(ctx) generates PGN from ctx.moves with default headers. But to properly generate PGN (with castling notation, capture notation, piece disambiguation), we need board state before each move.
I think the intention is: after DefaultRules.applyMove is called repeatedly (in PgnParser.importGameContext), the final GameContext has all moves but we lose the intermediate states. To export, we'd need to replay.
For pragmatism and to keep the plan simple: make exportGameContext replay the moves from initial position to reconstruct the game, then export via the existing logic.
- Step 2: Implement exportGameContext with replay
def exportGameContext(context: GameContext): String =
val headers = Map(
"Event" -> "?",
"White" -> "?",
"Black" -> "?",
"Result" -> "*"
)
// Replay all moves from initial position to get HistoryMove records
val historyMoves = scala.collection.mutable.ListBuffer[HistoryMove]()
var ctx = GameContext.initial
for move <- context.moves do
val color = ctx.turn
val pieceType = ctx.board.pieceAt(move.from).map(_.pieceType).getOrElse(PieceType.Pawn)
val isCapture = ctx.board.pieceAt(move.to).isDefined || move.moveType == MoveType.EnPassant
val castleSide = move.moveType match
case MoveType.CastleKingside => Some("Kingside")
case MoveType.CastleQueenside => Some("Queenside")
case _ => None
val promotionPiece = move.moveType match
case MoveType.Promotion(pp) => Some(pp)
case _ => None
historyMoves += HistoryMove(move.from, move.to, castleSide, promotionPiece, pieceType, isCapture)
ctx = DefaultRules.applyMove(ctx, move)
val history = GameHistory(historyMoves.toList, context.halfMoveClock)
exportGame(headers, history)
- Step 3: Ensure imports
import de.nowchess.io.GameContextExport
import de.nowchess.api.move.MoveType
import de.nowchess.api.board.PieceType
import de.nowchess.api.game.GameContext
import de.nowchess.rules.sets.DefaultRules
- Step 4: Run PgnExporterTest
./gradlew :modules:io:test --tests "de.nowchess.io.pgn.PgnExporterTest" -v
Expected: All existing tests still pass.
Task 7: Add exportGameContext tests to PgnExporterTest
Files:
-
Modify:
modules/io/src/test/scala/de/nowchess/io/pgn/PgnExporterTest.scala -
Step 1: Add test for round-trip (import then export)
test("exportGameContext: round-trip import->export preserves moves") {
val pgn = """[Event "Test"]
[White "A"]
[Black "B"]
1. e4 e5 2. Nf3 Nc6
"""
val importResult = PgnParser.importGameContext(pgn)
importResult.isRight shouldBe true
val ctx = importResult.getOrElse(???)
val exported = PgnExporter.exportGameContext(ctx)
exported.contains("1. e4 e5") shouldBe true
exported.contains("2. Nf3 Nc6") shouldBe true
}
- Step 2: Add test for empty context
test("exportGameContext: empty game returns headers only") {
val ctx = GameContext.initial
val exported = PgnExporter.exportGameContext(ctx)
exported.contains("[Event") shouldBe true
exported.contains("*") shouldBe true // Result terminator
}
- Step 3: Run tests
./gradlew :modules:io:test --tests "de.nowchess.io.pgn.PgnExporterTest" -v
Expected: All tests pass.
Task 8: Update GameEngine to add loadGame and exportGame
Files:
-
Modify:
modules/core/src/main/scala/de/nowchess/chess/engine/GameEngine.scala -
Step 1: Add loadGame method
Replace loadPgn:
def loadGame(importer: GameContextImport, input: String): Either[String, Unit] = synchronized {
importer.importGameContext(input) match
case Left(err) => Left(err)
case Right(ctx) =>
val savedContext = currentContext
currentContext = GameContext.initial
pendingPromotion = None
invoker.clear()
var error: Option[String] = None
if ctx.moves.isEmpty then
// No moves: just load the position
currentContext = ctx
notifyObservers(BoardResetEvent(ctx))
Right(())
else
// Replay moves through the command system
ctx.moves.foreach: move =>
handleParsedMove(move.from, move.to)
move.moveType match
case MoveType.Promotion(pp) => completePromotion(pp)
case _ => ()
if pendingPromotion.isDefined && move.moveType != MoveType.Promotion(_) then
error = Some(s"Promotion required for move ${move.from}${move.to}")
error match
case Some(err) =>
currentContext = savedContext
Left(err)
case None =>
notifyObservers(PgnLoadedEvent(currentContext))
Right(())
}
- Step 2: Add exportGame method
def exportGame(exporter: GameContextExport): String = synchronized {
exporter.exportGameContext(currentContext)
}
- Step 3: Remove loadPgn method
Delete the existing loadPgn entirely.
- Step 4: Ensure imports
At top of GameEngine:
import de.nowchess.io.{GameContextImport, GameContextExport}
import de.nowchess.api.move.MoveType
Task 9: Update GameEngineLoadPgnTest
Files:
-
Modify:
modules/core/src/test/scala/de/nowchess/chess/engine/GameEngineLoadPgnTest.scala -
Step 1: Update test "loadPgn: valid PGN"
Change:
engine.loadPgn(pgn) shouldBe Right(())
To:
engine.loadGame(PgnParser, pgn) shouldBe Right(())
And add import:
import de.nowchess.io.pgn.PgnParser
- Step 2: Bulk replace loadPgn calls
Replace all engine.loadPgn( with engine.loadGame(PgnParser,
Affected lines (approx):
-
23, 32, 38, 48, 58, 74, 80, 145, 146
-
Step 3: Add test for FEN loading
test("loadGame(FenParser): sets position without replaying") {
val engine = new GameEngine()
val fen = "8/4P3/4k3/8/8/8/8/8 w - - 0 1"
val result = engine.loadGame(FenParser, fen)
result shouldBe Right(())
engine.context.moves.isEmpty shouldBe true
engine.board.pieceAt(Square(File.E, Rank.R7)) shouldBe Some(Piece.WhitePawn)
}
And add import:
import de.nowchess.io.fen.FenParser
- Step 4: Add test for exportGame
test("exportGame(FenExporter): exports current position as FEN") {
val engine = new GameEngine()
engine.processUserInput("e2e4")
val fen = engine.exportGame(FenExporter)
fen.contains("e4") shouldBe false // FEN is position format, not notation
fen.contains("P") shouldBe true // Should have pawn symbol
}
test("exportGame(PgnExporter): exports as PGN with moves") {
val engine = new GameEngine()
engine.processUserInput("e2e4")
engine.processUserInput("e7e5")
val pgn = engine.exportGame(PgnExporter)
pgn.contains("e4") shouldBe true
pgn.contains("e5") shouldBe true
}
And add imports:
import de.nowchess.io.fen.FenExporter
import de.nowchess.io.pgn.PgnExporter
- Step 5: Run tests
./gradlew :modules:core:test --tests "de.nowchess.chess.engine.GameEngineLoadPgnTest" -v
Expected: All tests pass.
Task 10: Full build and test
Files:
-
All modules
-
Step 1: Build all
./gradlew build
Expected: GREEN (no errors, no test failures).
- Step 2: Check coverage
python3 jacoco-reporter/scoverage_coverage_gaps.py modules/io/build/reports/scoverageTest/scoverage.xml
Expected: No gaps in new code.
- Step 3: Commit all changes
git add -A
git commit -m "refactor(io): unify import/export interfaces with Either and GameContext"
Summary
After these 10 tasks:
GameContextImportnow returnsEither[String, GameContext]with error messagesFenParser,PgnParserboth implementGameContextImportPgnExporterimplementsGameContextExportand can export fromGameContext.movesGameEngine.loadGame(importer, input)handles any format uniformlyGameEngine.exportGame(exporter)exports to any format- All tests updated and passing
- No breaking changes to public API (only import/export interfaces changed as designed)