Files
NowChessSystems/docs/superpowers/plans/2026-04-05-io-interface-refactor.md
T
Janis f6f05ff2a1 refactor(io): change GameContextImport return type from Option to Either
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>
2026-04-05 12:33:13 +02:00

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.isDefinedcontext.isRight; context.getcontext.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 falsecontext.isLeft shouldBe true

  • Line 95: context.isDefined shouldBe falsecontext.isLeft shouldBe true

  • Line 101: context.isDefined shouldBe falsecontext.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:

  • GameContextImport now returns Either[String, GameContext] with error messages
  • FenParser, PgnParser both implement GameContextImport
  • PgnExporter implements GameContextExport and can export from GameContext.moves
  • GameEngine.loadGame(importer, input) handles any format uniformly
  • GameEngine.exportGame(exporter) exports to any format
  • All tests updated and passing
  • No breaking changes to public API (only import/export interfaces changed as designed)