# GWDGUI code review — commit 98da11d (baseline)

Reviewer: Claude (format: /tmp2/gwies/zzzzzz4/claude)
Date: 2026-10-24
Scope: the whole committed tree at `98da11d` ("Remove duplicate user manual text file"),
about 31,500 lines of Pascal in 50 units, plus `tests/`, `MANUAL.md`, `protocol.txt` and
`DXP Protocol.md`. The untracked `GWD/` folder (the older GUI) is not part of this review.
The working tree was clean.

GWDGUI has no revision counter, so this review is named after the commit hash.

**Focus, as requested:** whether the game tree is the single source of truth, and whether
there is hidden state outside it. That has its own section below, before the findings.

## Issue labels

Every finding has a stable ID: an area letter plus a number. IDs are shared across all
reviews: an issue keeps its ID until it is resolved, and a number is never reused. The
status table at the end lists every ID known at this commit. It is sorted with open
issues first (Open, New, Partly or Mostly fixed), then closed ones (Fixed, Closed,
Won't fix); within each group by severity (High → Low), then priority (P1 → P3),
then ID.

| Letter | Area |
|---|---|
| G | Game tree: `uGameTree`, `uGameTreeActor`, `uGameTreeBuilder`, `uGameTreePdn` |
| U | Main window and GUI state: `uMainForm`, forms, board control |
| R | Rules, board, FEN and move notation: `uDraughtsBoard` |
| M | Match running: `uGameOrchestrator`, `uGameRunnerThread`, clocks |
| D | DXP and Hub engines, processes |
| A | Analysis: annotator, MCTS, Auto Play, PV browser, score history |
| P | PDN and database |
| T | Tournament |
| F | Speed (performance) and memory |
| C | Configuration, build, tests, repository |

- **Severity** (impact): High = crashes, hangs, illegal moves accepted, lost or corrupted
  games, analysis or results, or an engine out of step with the game; Medium = edge-case
  errors, wrong but recoverable behaviour, or a sizeable slowdown; Low = minor, cosmetic,
  or a future risk.
- **Priority** (when to fix): P1 = before the next feature; P2 = within the next few
  commits; P3 = when convenient, or a design decision.

## How this was checked

- **Build:** `lazbuild gwdgui.lpi` with Lazarus 3.0 and FPC 3.2.2 on Linux (gtk2): no
  warnings in GWDGUI's own units. All 32 warnings come from the LCL. The build was done on
  a copy outside your folder, because the linked machine has no FPC installed.
- **`tests/run_regression_checks.sh`: all 5 checks and the source audit pass.**
- **Probe programs.** They drive the actor and the tree directly, plus the real
  `TMainForm` under Xvfb, with its private section made public in a throwaway copy. They
  reproduce G1–G5, U2 and P1. The sources are in `claude/probes-98da11d/`.
- **Rules:**
  - Perft from the start position matches the published values to depth 9 (41,022,423).
  - Woldouby matches to depth 12.
  - Move sets agree with an independent Python generator on 80,000 random king-heavy
    positions, with 0 mismatches.
- **Engines:** fake Hub and DXP engines (small Python scripts) were used to reproduce A1,
  D1, D2 and D4.
- **Code read:** every unit. The engine, PDN/database/tournament and rules/PV areas were
  each read in full by a separate review agent; I checked their main claims against the
  code.

## Is the game tree the source of truth?

**Short answer: not yet.** The *display* is derived from the tree: board, move list, PV
panes, clocks, score graph and PDN export all read `TGameTree`. Mutations go through
`TGameTreeActor`, with a grep-based audit test. That is a solid base. But there is real
state in five other places, and several of the High findings come from it:

1. **Inside the tree itself.** `TGameTree` keeps 18 side tables:
   - per-ply engine, annotator and Auto Play score/PV/depth/time and per-ply clocks, keyed
     by **ply number**;
   - the same data for variations, keyed by a **variation key string that contains the
     index of the line** in `FVariationLines`.

   The node fields are only copies, refreshed by `ApplyStoredAnalysis` and
   `ApplyStoredVariationAnalysis`. The tables survive mainline replacement, trimming and
   `ClearStoredVariations`, so data is re-attached to *different moves* (G3). Variations
   exist twice: as text lines in `FVariationLines` and as nodes, and some paths update
   only the text (G1).
2. **In the runner thread, for engine games.** While a game runs, the authority is
   `TGameOrchestrator` (its board and `TGameMoveHistory`). The tree is a projection rebuilt
   from snapshots (`UpdateRunningGameTreeFromSnapshot`). It becomes authoritative once the
   game stops. That split is reasonable, but the history holds per-move PVs and times that
   never reach the tree (M1).
3. **In the main form.**
   - The cursor is `FGameBrowsePly`, `FVariationIndex`, `FVariationBrowsePly` and
     `FVariationBrowseMoves`, i.e. ply and index numbers, not a node ID.
   - Which tree is shown is decided by `FGameActive` plus the games list box's
     `ItemIndex`, a widget.
   - `FBoard` is rebuilt by a different, unvalidated replay from the one used everywhere
     else (U2).
   - `PrepareGameTreeForPdn` writes the read-only header edit boxes back into the tree.
4. **In the copies of the tree's own data.** `CopyStoredVariationsToCommand` takes the
   current variation lines when a rebuild command is *created* and re-adds them after the
   rebuild. So a new game inherits the previous game's variations (G4).
5. **Outside the GUI.** These also hold state that can disagree with the game records:
   - the tournament grid and JSON (T1, T2);
   - the engines' own idea of the position (D1, A1);
   - the PDN files, through lossy import (P1–P5).

The tree is also not structurally a tree (G7). Every mainline node is a child of the
root. A variation hangs under the node of its *position* when made in the GUI, but under
the *replaced move* when imported from PDN. Positions are rebuilt from ply numbers and
parsed keys, not from parent links.

**Suggested direction.** Make nodes own their data: analysis, clocks and comments as node
fields, with no side tables keyed by ply or index. Give each node a real parent (the
position before its move) and ordered children (the first child is the main
continuation). Keep the cursor as a node ID. Then:
- a variation is just a non-first child;
- "add move" means find-or-create a child;
- rebuilds from a runner snapshot update the first-child chain;
- PDN import builds the same structure recursively (including nested variations);
- every board is `replay(path from root to node)` with validation.

Most of G1–G7, U2, U3 and P1/P3 then disappear by construction.

## Summary

**Good:**
- **Move generator:** it is exactly right (perft to depth 9, 0 mismatches against a
  reference generator). Captures with the same from and to squares are kept apart, and an
  ambiguous short form is refused.
- **Engine moves:** every one is checked by the GUI's own generator before it is played.
  Hub engines get the full move list on every search.
- **Threading:** one worker thread per engine, with message queues. `TThreadMessageQueue`
  and `TCancellationToken` are small and correct. Snapshots are coalesced on the GUI side.
- **Actor:** commands carry tree, node and search IDs. Analysis results for a node that
  no longer exists are rejected, and a search whose target disappears is marked failed.
- **Database:** SQL uses bound parameters throughout. Position search uses a canonical
  key from the replayed board, and the bulk import is batched with duplicate detection.
- **Tournament:** scoring, Sonneborn-Berger and byes are correct and separate from the
  GUI.

**Main concerns, in order:**
1. **Variations and annotations are lost or damaged in normal use (G1–G5, P1–P2).**
   - Variations from a PDN file are invisible and dropped on save.
   - Playing a move inside a variation cuts the rest of it off.
   - Analysis stays on the ply after the move there changes.
   - A pasted FEN inherits the previous game's variations.
2. **The annotator writes the previous position's PV onto the new node whenever you
   browse (A1).**
3. **Engine handling (D1–D4).**
   - A late `done` becomes the next move.
   - A DXP crash can kill the GUI with SIGPIPE.
   - Every move is cut off at 60 s, whatever the time control.
   - A DXP engine's "I win" becomes the result.
4. **An unsaved PDN or FEN game disappears when a game is started, selected or removed
   (U1).** Browsing past a short-form multi-capture raises an exception (U2).
5. **Tournament results live only in an unsaved grid that can drift from the PDN
   (T1, T2).**

## Findings

### High

**M3 · High · P1 — Moves added right after Stop are wiped by the runner's next snapshot.**
- **Sequence:**
  1. `MarkGameStopped` marks the GUI copy of the snapshot, and so the tree, as stopped
     straight away.
  2. `PlayHumanMove` then lets the user append to the mainline (uMainForm.pas:2767-2776).
  3. The orchestrator never sets `gosStopped` itself; its `ChangeState` only sets running,
     game over and error. So a snapshot the runner publishes while finishing its search
     still says `gosRunning`.
  4. `RunningGameTreeExtendsSnapshot` only protects a tree that is ahead of the snapshot
     when the *snapshot* says stopped or game over (4554). So the tree is rebuilt from the
     snapshot (tier 3 of `UpdateRunningGameTreeFromSnapshot`), and the user's moves are
     gone.
- **Verified:** read (found in review 01; I re-traced it at this commit).
- **Fix:** keep the tree read-only for edits until the lifecycle is `rglFinished`, or
  have the runner report `gosStopped` itself. Remove the full-rebuild path for running
  games (see M4).

**M4 · High · P1 — Pressing Stop just after a game ends can replace the real result with
`*`.**
- **Sequence:** `TRunningGame.FSnapshot` is the last runner snapshot, kept for the game's
  lifetime. The GUI edits it in place: `MarkStopped` sets `gosStopped` and result `*`.
  Then `UpdateRunningGameTreeFromSnapshot(AGame)` writes it into the tree.
  - Stop on a game that has reached `gosGameOver`, but whose runner hasn't finished, sets
    the tree's result to `*`. If the runner's final snapshot has already arrived, nothing
    restores the result.
  - The tournament grid is updated from the same game (T2).
- **Related:** the same stored snapshot is re-applied after every analyzer PV, and on PDN
  export (`RunningGameTreePdnText`), so a read path writes the tree.
- **Verified:** read (from review 01; re-traced).
- **Fix:** don't use the stored snapshot as a sync source. Make Stop a state change on the
  tree that never touches the result once the game is over, and make export side-effect
  free.

**G1 · High · P1 — Variations imported from PDN never become tree nodes: they are
invisible and are dropped by Save PDN.**
- **Code:** `LoadPdnGameToDisplay` calls `LoadTreeVariationsFromLegacyText`. That stores
  each line with `gtcStoreVariationLine` (`StoreTreeVariation(..., False)`), which only
  adds text to `FVariationLines`. Nodes are only made by `RecordGameTreeVariationLine`,
  which this path never calls. Rendering and export walk nodes.
- **GUI probe:** open `1. 32-28 (1. 33-28 18-23) 18-23 (1... 19-23 2. 37-32) 2. 37-32`.
  The tree has `variation lines=2`, but 0 variation nodes. The move list shows
  `1.32-28 18-23 2.37-32`, and Save PDN writes the same, without the variations.
- **Manual:** §8.1 says "the loader reconstructs ... PDN variations in the game tree".
- **Tests:** they pass because they call `RecordGameTreeVariationLine` by hand after
  `StoreVariationLine` (`tests/game_tree_pdn_filter_check.pas:529`).
- **Fix:** make storing a line and materialising its nodes one actor operation. Better,
  drop the text form (see the suggested direction above).

**G2 · High · P1 — Adding a move in a variation silently truncates the existing line.**
- **Inside a variation:** `StoreMoveVariationFromNode` builds "moves up to this ply + new
  move" and overwrites the line with `StoreVariationLineAtIndex`.
  - Probe: variation `19-23 37-32 14-19 41-37`; play 31-27 after its second move. The line
    becomes `19-23 37-32 31-27`, and 14-19 41-37 are gone.
  - Playing the *same* next move also truncates the line to that point.
- **From the mainline:** `FindStoredVariation` matches an existing line by its first move.
  - Probe: replaying 19-23 from ply 1 turns `19-23 37-32 14-19` into `19-23`.
- **Missing check:** there is no check against the mainline either. Playing the game's own
  next move from an earlier ply stores a duplicate variation (`1. 32-28 (1... 18-23)
  18-23`; probe 3).
- **Fix:** adding a move should find or create a child of the current node. Existing
  continuations become siblings (sub-variations), never truncated. If the move equals an
  existing child, including the mainline move, just go to that child.

**G3 · High · P1 — Analysis and clocks are stored by ply number or variation index, so
they re-attach to different moves.**
- **Mainline:** `RecordMainlineMove` with a different move at ply N removes the old node,
  then `AddMainlineMove` calls `ApplyStoredAnalysis(N)` and `ApplyStoredClockState(N)`.
  - Probe H1: ply 2 was 18-23 with annotator PV `37-32 12-18` (0.55) and engine score
    0.44. After a rebuild with 19-23 at ply 2, the new 19-23 node shows the same PV, 0.55
    and 0.44.
  - `gtcAddMainlineMove` at an existing ply does the same.
- **Variations:** `ClearStoredVariations` removes the lines and nodes but not the nine
  `FVariation*ByKey` tables.
  - Probe H3: an annotator PV `34-29 23x34` (−1.20) on variation 0 (19-23) reappears on a
    new, different variation 0 (17-21).
- **Rewritten lines:** a rewritten variation line (G2) gives its replacement move the
  overwritten move's analysis, through `ApplyStoredVariationAnalysis`.
- **Clocks twice:** they are stored at tree level (`FWhiteRemainingSeconds` and friends)
  as well as per node, and `RecordClockState` writes both.
- **Effect:** wrong analysis is displayed and exported to PDN as comments.
- **Fix:** store analysis, clocks and comments on the node only, and delete them with the
  node. Until then, clear the per-ply tables from N onwards in `RemoveMainlineFrom`, and
  clear the per-key tables in `ClearStoredVariations`.

**G4 · High · P1 — A rebuild re-adds the previous game's variations to the new game.**
- **Code:** `CreateRebuildFromInputsCommand` copies the tree's current lines into the
  command (`CopyStoredVariationsToCommand`). `BeginRebuild` with a new signature clears
  everything, then `RebuildStoredVariations` stores the old lines again.
- **Probe H2:** a root variation 33-28 in game A survives `LoadStandaloneFen('W:W31,32:B19,20')`
  as a child of the new root, where 33-28 is not even a legal move.
- **GUI probe:** after Open PDN and then Paste FEN, the tree still has 2 variation lines,
  and `VariationLine[0]` is still the previous game's `33-28 18-23`.
- **Fix:** a rebuild for a different game must start empty. Carry variations across a
  rebuild only for the same game, and only after checking they still fit the mainline.

**U1 · High · P1 — An unsaved PDN or FEN game becomes unreachable, or is wiped, when an
engine game is started, selected or removed.**
- **Code:** the standalone tree `FGameTree` is shown only while `FGameActive = False`.
  - Selecting a game in the list, starting a game, or a tournament starting its next game
    (`StartGameFromSetup`) sets `FGameActive := True`. There is no way back to the
    standalone tree.
  - Removing the last game calls `ApplyGameToDisplay(nil)`, then
    `LoadStandaloneFen(start)`, which overwrites it.
  - The close prompt (`HasDisplayedGameToSave`) only looks at the displayed tree.
- **Scenario:** a PDN game you have annotated is lost without a warning as soon as a
  tournament schedules its next game.
- **Verified:** read.
- **Fix:** make the standalone game an entry in the games list (or give it a "Board"
  tab). Only replace it with a "save changes?" prompt.

**U2 · High · P1 — The main board replays moves without legality checks, so a short-form
multiple capture raises, and the board can disagree with every other view.**
- **Two replay paths:**
  - `UpdateBoardFromBrowseState` builds `FBoard` with `PlayMove(..., True, False)` (no
    validation).
  - The legal-move list, highlights, analyzer, FEN and database search use
    `TryBuildTreeBoardAtNode` (validated).
- **Why short form breaks:** PDN import validates the moves, but stores the *raw* tokens
  in the tree. Without validation, `37x17` can't be resolved into `37x17x32x22`.
- **GUI probe:** with `[FEN "W:W37,45:B32,22,5"] 1. 37x17 5-10 2. 45-40`, the game loads.
  Browsing to the end raises `EInvalidOperation: No captured piece between 37 and 17`.
  Short form is normal in PDN from other programs.
- **Related:** R2 (the failed move leaves the board half-changed) and R3.
- **Fix:** have one validated `BoardAtNode` and use it everywhere. Store moves in the
  tree in the canonical form returned by `TryGetLegalMove`.

**A1 · High · P1 — While you browse, the annotator stores the previous position's PV and
score as the new node's analysis.**
- **Code:** `TAnalyzerRunnerThread.HandleCommand(accAnalyze)` switches the PV context
  (new base board, new search ref) *before* the engine worker has stopped. The engine's PV
  is only cleared in `SetGamePosition`. The first log line after the switch is
  `send; message=stop`, and `RuntimeLog` then publishes the old PV with the new search
  ref. `RecordAnalyzerSnapshotInGameTree` stores it on the new node, and nothing checks
  that the PV is legal from that position.
- **Fake Hub engine:** analysing B (after 32-28) first publishes A's PV `32-28 19-23`
  (0.25, depth 9) tagged with search 2 and board B; its first move is illegal there. The
  terminal branch publishes the stale PV twice, because it never calls `SetGamePosition`.
- **Scenario:** stepping through a game faster than the annotator can produce a new PV
  leaves each node with its predecessor's PV. That ends up in saved PDN.
- **Fix:** clear PV, score, depth and time when stopping, and before the terminal publish.
  Switch the context only after `SetGamePosition`, or tag info lines with a search
  generation. Reject any PV whose first move is not legal from the node's position, in
  `RecordAnalyzerSnapshotInGameTree`.

**D1 · High · P1 — After a slow `stop`, the old search's `done move=` is played as the
answer to the next search.**
- **Code:** `WaitForSearchStopped` waits 2 s (`uHubEngine.pas:740`), then sets the state
  back to Ready anyway (962). The GUI sends `pos`, `level` and `go think` while the engine
  is still searching, which protocol.txt:153 does not allow.
- **Fake engine** needing 3 s to stop: the move for search 2 was the old position's move.
- **Effect:** the orchestrator rejects it if it's illegal, which aborts the game. If it
  happens to be legal, Auto Play or a match gets a move for the wrong position.
- **Fix:** wait for `done` (or a `ping`/`pong` fence) with a long timeout, restart the
  engine when it expires, and never send `pos` while Stopping.

**D2 · High · P1 — On Unix, a crashed DXP engine can kill the GUI with SIGPIPE.**
- **Code:** nothing ignores SIGPIPE, and `ssockets` sends without `MSG_NOSIGNAL`.
  `LocalSocketOpen` (`uDxpEngine.pas:805`) still reports the socket as open, so `DoStop`
  (1348) writes GAMEEND to a dead peer.
- **Test harness:** it exited with status 141 during engine shutdown. In the LCL GUI this
  is not verified, but LCL installs no handler either.
- **Fix:** `fpSignal(SIGPIPE, SIG_IGN)` at startup, and/or `MSG_NOSIGNAL`.

**D3 · High · P1 — Every engine move has a fixed 60 s limit, whatever the time control.**
- **Code:** `DxpMoveTimeoutMs = 60000` (`uDxpEngine.pas:217`, 1335) and
  `WaitForDoneMove(60000, ...)` (`uHubEngine.pas:939`). An empty result ends the game as
  `*` (`uGameOrchestrator.pas:530-537`). The Hub engine is not even sent `stop`.
- **Scenario:** at 90 min / 75 moves, the first long think aborts the game. With 5 s on
  the clock, a hung engine still gets 60 s.
- **Related:** `ReadDxpPacket` also ignores EOF, so a dead DXP engine is only noticed
  after those 60 s (D5).
- **Fix:** derive the deadline from the remaining clock plus increment and a margin, and
  score a timeout as a loss on time.

**D4 · High · P1 — A DXP engine's GAMEEND claim ("I win", "draw") becomes the official
result.**
- **Code:** `TranslateGameEndResult` (`uDxpEngine.pas:1012-1034`) is used as the game
  result (`uGameOrchestrator.pas:532`). In the DXP protocol the reason is informational.
- **Fake engine:** it sends E30 in the initial position. The result is recorded as 2-0,
  with 9 legal moves on the board.
- **Fix:** accept a resignation. Check draw claims against the GUI's own rules, accept a
  win claim only in a terminal position or on a flag fall, and keep the claim as a
  comment.

**R1 · High · P1 — `CurrentFEN` loses king status when a side has two or more kings.**
- **Code:** `AppendPieces(pkWhiteKing, 'WK')` writes `WK32,33,34`, but the parser applies
  K to one entry only (`uDraughtsBoard.pas:472-475`). Round trip: 3 white kings come back
  as 1 king and 2 men.
- **Affected:** Apply in Setup position, Copy FEN, the FEN field and the database search
  FEN. With men present the output is also non-standard: `W:W33:WK32:B1` has two W
  sections.
- **Tests:** they use one king per side only.
- **Fix:** write one section per colour with `K` on each king entry (`W:W31,K32,K33:B1,K2`),
  and add a test with several kings per side.

**P1 · High · P1 — Standard PDN variations are imported from the wrong position, and
GWDGUI writes them in a non-standard place.**
- **Import:** in `ProcessVariation` (`uPdn.pas:642-648`) the base ply from
  `InferVariationBasePly` is always overwritten by
  `StrToIntDef(ExtractAnnotationValue(..., 'base_ply'), LSlot)` when the variation has
  annotations, which it practically always does. So base ply = slot.
  - Standard PDN puts a variation *after the move it replaces*:
    `1. 32-28 18-23 (1... 19-23)`. That imports as slot 2, base 2, and is replayed
    *after* 18-23 as a White move "2. 19-23", which is illegal.
  - GUI probe: `(1. 33-28 18-23)` after 32-28 imports as `1|1`, i.e. as a reply to
    32-28.
- **Export:** `uGameTreePdn` writes a GUI-made variation (`DisplayAfterPly = BasePly`) in
  front of the move it replaces: `1. 32-28 (1... 19-23) 18-23`. Other PDN programs read
  that as an alternative to 32-28.
- **Two conventions:** the legacy `PdnGameToText` already uses the standard placement
  (test at `tests/pdn_regression_check.pas:212-218`).
- **Effect:** PDN files exchanged with other programs are misread in both directions.
- **Fix:**
  - Import: use the move number when it is present; place the variation as an
    alternative to the preceding move.
  - Export: write each variation right after the move it replaces.
  - Add a round-trip test with a file in standard form.

**P2 · High · P2 — Free-text PDN comments are discarded on import.**
- **Code:** only comments containing a score, `E:` or `A:` are kept (`uPdn.pas:569-579`,
  395-400). Comments before the first move are dropped too. The regression test asserts
  this behaviour (`tests/pdn_regression_check.pas:50`, 69).
- **Effect:** opening and re-saving an annotated game, or importing it into the database,
  loses all the commentary. The tree already has `CommentText`; the importer just never
  fills it.

**T1 · High · P1 — Tournament standings exist only in the in-memory grid.**
- **Code:** the JSON is written only by Save or by the close prompt
  (`uTournamentDialog.pas:486-509`, 1093-1148). PDN records carry no round, game number or
  tournament ID, so the grid can't be rebuilt from the games.
- **Scenario:** a crash during a long tournament loses all standings.
- **Fix:**
  - Autosave atomically (temp file plus rename) after every result.
  - Write `[Round]`, `[GameNo]` and a tournament ID into each PDN record.
  - Better: derive the grid from the stored games.

**T2 · High · P1 — Tournament grid results can silently disagree with the game records.**
- **Ways it happens:**
  - (a) A manual result on a played game keeps the reason "Game finished" (1023-1040),
    and the PDN keeps the original result.
  - (b) Reschedule appends a second PDN record for the same pairing.
  - (c) Removing an old finished game from the list calls `GameStopped`. That matches by
    GameId without checking whether the entry is active (1441-1457), re-applies the old
    result over a manual override or a rescheduled game, and appends the game to the PDN
    again.
  - (d) A finished game with no moves gets a result but no PDN record.
  - (e) Saving while games run and resuming replays them, so they are duplicated in the
    PDN.
- **Verified:** read; (c) confirmed in the code.
- **Fix:** derive results from the game records. Keep a manual override as an explicit
  field, and ignore `GameStopped` for entries that are no longer active.

### Medium

**G5 · Medium · P1 — Variations at the root are stored and shown on the board, but never
in the move list or PDN. Moves can't be played at the end of a pasted position.**
- **Root variations:** `RenderMainline` only renders the children of mainline nodes, never
  those of the root.
  - Probe: a root variation 33-28 is accepted, but the export is
    `1. 32-28 18-23 2. 37-32`.
- **Playing at the end:** on the standalone tree, `PlayHumanMove` only appends when there
  is a running game (`LGame <> nil`). At the end of a pasted position
  (`FGameBrowsePly = -1`), clicking a piece does nothing.
- **Only remaining route:** press Left to reach ply 0, then play. The moves go into an
  invisible root variation.
  - GUI probe: after `32-28 18-23` the board shows the position, but the move list says
    `(none)` and Save PDN writes no moves.
- **Fix:** render root children (as variations to move 1). On the standalone tree,
  playing at the end of the mainline should extend it.

**G8 · Medium · P2 — The score-annotation text projection fills gaps forward and is fed
back into the tree, so an annotator score creeps onto later plies.**
- **Code:** `GameTreeScoreAnnotationsText` (`uGameTreePdn.pas:700-702`) uses the previous
  ply's annotator score when a ply has none. `PreserveExistingSnapshotAnnotations`
  (uMainForm.pas:5303-5327) copies those lines into empty slots of a runner snapshot, and
  the result is used to rebuild the tree. The same text also serves as the equality test
  for "tree matches snapshot" (5286, 5419).
- **Probe:** annotator score 0.5 on ply 1 only. After one rebuild cycle, plies 2-4 carry
  `A: 0.5`.
- **Fix:** keep the projection output-only, and compare node data, not rendered text.

**G6 · Medium · P2 — The tree accepts any text as a move, at any ply.**
- **Code:** the actor and tree never validate. `gtcAddMainlineMove` at ply 5 on an empty
  tree is accepted, leaving nil holes. `MainlineCount` is then 5 but `CopyMainlineMovesTo`
  returns 1 move (probe H9). PDN variations are stored without any legality check (see P1
  for how wrong they can be).
- **Effect:** a bad move only shows up later, as a board that can't be built
  (`TryBuildTreeBoardAtNode` returns False) or as U2's exception.
- **Fix:** validate moves against the parent's position in the actor, rejecting illegal
  ones and storing the canonical form, and refuse holes.

**G7 · Medium · P2 — The game tree is not structurally a tree.**
- **Structure:**
  - All mainline nodes are children of the root: in the probe, `ply3.Parent = Root`.
  - A GUI-made variation hangs under the node of the position it starts from. A PDN
    variation hangs under the move it replaces.
  - Positions are rebuilt from `PlyNumber` and a parsed `BasePly:DisplayAfterPly:Index:Ply`
    key, so `Parent` is useless for walking back.
- **Keys:** the variation index in the key means deleting a line would renumber its
  neighbours. Nested variations can't be represented (P3).
- **Speed:** `FindNodeById` and `FindVariationNodeByKey` are recursive linear searches.
  `VariationNodeMoveCount` calls `FindVariationNode` in a loop.
- **Fix:** this is the root cause of G1–G5. See the suggested direction above.

**U3 · Medium · P2 — In a finished engine game, a move at the final position extends the
official mainline beyond the result.**
- **Code:** `PlayHumanMove` appends to the mainline of a stopped or game-over running game
  (`gosStopped`, `gosGameOver`). For a game drawn by repetition or the 25-move rule, or
  lost on time, the saved PDN then has `[Result "1/2-1/2"]` followed by extra moves.
  - After that, `RunningGameTreeExtendsSnapshot` makes the GUI ignore later runner
    snapshots, including a late final result.
- **Fix:** moves after the end of a finished game should become a variation, or the
  result should be reset to `*` with a warning.

**M1 · Medium · P2 — For engine games, the final PV and time of each move stay in the
orchestrator; the tree only gets a lossy sample.**
- **Code:** `TGameOrchestrator.PlayMove` stores the engine's PV and `SecondsUsed` per move
  in `FMoveHistory`, then clears the engines' PVs. `CreateSnapshot` sends only the
  side-to-move's *current* PV, and at most every 500 ms during thinking.
  - The GUI keeps only the newest snapshot per game per 50 ms tick.
  - `TryAppendSnapshotMoves` records the PV and clock for the *last* appended move only.
- **Effect:** in fast games many plies have no engine PV, or an intermediate one, and no
  clock. The real data is never exported.
- **Also:**
  - The PV of the search that produced move N+1 sits on node N, while its score annotation
    sits on node N+1.
  - Runner PVs are attached by ply number (`MainlineNode[PvBasePly]`,
    `TryResolveRunningSnapshotPvNode`). The snapshot's `PvBaseBoard` position key is never
    checked against that node.
- **Fix:** send the full per-move history (PV, score, depth, time, clocks) in the snapshot,
  or at least the entries added since the last snapshot.

**M2 · Medium · P2 — An illegal human move ends the game with an error instead of being
rejected.**
- **Code:** `grcHumanMove` calls `FOrchestrator.PlayMove`, which raises on an illegal
  move. `DrainCommands` has no handler, so `Execute` sets `FRunnerFailed` and the game
  goes to `gosError`.
- **How it happens:** the GUI checks legality against the tree, which lags the
  orchestrator by up to one snapshot. In a human-vs-human game, two quick board moves
  before the snapshot arrives send the second move against the wrong position.
- **Fix:** catch the exception, log it, and post a "move rejected" snapshot.

**R2 · Medium · P1 — `PlayMove` changes the board before its checks, and has no
rollback.**
- **Code:** the source square is emptied at `uDraughtsBoard.pas:815`, and captured
  squares at 830 and 840, before the checks at 817-820 and 837 can raise.
- **Example:** `PlayMove('46-41', True, False)` from the start raises, and 46 is now
  empty. Every unvalidated caller (U2, the PV browser A2) is left with a corrupted board
  after an exception.
- **Fix:** check everything on a copy of the squares, then commit.

**R3 · Medium · P2 — The unvalidated `PlayMove` path accepts wrong-side and impossible
moves.**
- **Code:** there is no check that the piece belongs to the side to move, nor any
  direction or adjacency check for quiet moves.
- **Examples:** from the start, `19-24` (a Black move) is played with White to move. On
  `W:W28:B1`, `28-33` (a man moving backwards), `28-6` and `28-29` are accepted.
- **Fix:** add those checks, or remove the unvalidated mode from the display paths (U2,
  A2).

**R4 · Medium · P2 — `LoadFromFEN` sets the side to move before parsing succeeds.**
- **Code:** `FSideToMove` is assigned at 432-440; the squares only at 479-482.
- **Example:** after a failed `B:W99:B1` the board says Black to move, but keeps White's
  9 legal moves. In Setup position, the radio group and the board then disagree, and
  Apply sends a `B:` FEN the user never chose.
- **Fix:** parse into locals and commit everything together.

**R5 · Medium · P2 — PDN captures written with landing squares are rejected, and exports
use captured-square lists.**
- **Import:** `uDraughtsBoard.pas:318-320` requires the second square to be the final
  destination. So a path like `32x21x12` (intermediate landing squares) for the only legal
  capture `32x12x27x17` is refused, and the game stops loading there.
- **Export:** it writes the internal `from x to x captured...` form. Other programs may
  read that as a path; this was not tested against another program.
- **Fix:** also accept landing-square paths. Export `from x to`, adding squares only to
  resolve an ambiguity.

**A2 · Medium · P2 — The PV browser silently shows cut-off or impossible boards.**
- **Code:** `uPvBrowser.pas:214-227` replays the PV without validation, and stops silently
  at the first exception.
- **Examples:**
  - `32-28 19-23 46-41 23x32`: the mini board loses White's piece on 46 (R2), with the
    rest of the PV dropped.
  - `32-28 28-23 19-24` (White moving twice): the board shows a position that can't
    occur.
- **Fix:** validate while replaying, and mark the first illegal ply in the memo.

**A3 · Medium · P1 — While the PV browser is frozen, the shared mini board is overwritten
with the game position.**
- **Code:** the browser draws into `FMiniBoard`, but `RefreshDisplayFromGameTree` copies
  the main board into it (`uMainForm.pas:7659`). While browsing is locked,
  `SetData` only stores pending data and does not redraw (`uPvBrowser.pas:411-413`).
- **Scenario:** click PV move 3 during a running game. On the next refresh the mini board
  shows the game position, while the memo still highlights ply 3.
- **Verified:** by following the calls in the code; not verified at runtime.
- **Fix:** give the browser its own board, or don't copy into it while it is locked.

**P3 · Medium · P2 — Nested variations (a variation inside a variation) are dropped on
import.**
- **Code:** `ProcessVariation` calls `ExtractPdnMoveData` with nil variation lists.
- **Probe H6:** `(1... 19-23 2. 37-32 (2. 31-27 14-19) 14-19)` keeps only the outer line,
  `19-23 37-32 14-19`.
- **Fix:** follows from G7: build nested children recursively.

**P4 · Medium · P2 — A comment spanning several lines shifts annotations onto the wrong
moves.**
- **Code:** the comment keeps its newline (`uPdn.pas:573-578`), but annotations are stored
  as one line per ply (829, 831).
- **Example:** `{E: 0.1⏎A: nice} 18-23 2. 37-32 {E: 0.3}` gives 18-23 "A: nice", and 37-32
  loses its 0.3. The database stores the shifted scores too.
- **Fix:** turn CR/LF into spaces inside comments.

**P5 · Medium · P2 — The result token in the movetext is ignored.**
- **Code:** only the `[Result]` tag is used (`uPdn.pas:559`, 794).
- **Effect:** a game ending `... 2-0` without a tag imports as `*`, so database statistics
  undercount, and a tag that contradicts the movetext isn't noticed.

**P6 · Medium · P2 — `0-0` is parsed as a move.**
- **Code:** `IsPdnResultToken` doesn't know it (114-120), and `LooksLikePdnMove` accepts it
  (221-225).
- **Effect:** in a database import it becomes "Illegal move", and the whole import aborts
  (P8).

**P7 · Medium · P2 — Game boundaries depend only on a `[` at the start of a line.**
- **Code:** `uPdn.pas:871-878`, 914-931; tag values are taken between the first and last
  quote (779-786).
- **Effects:**
  - A tags-only game (a forfeit) merges into the next game.
  - Untagged games become one game.
  - A comment line starting with `[` splits a game.
  - Two tags on one line corrupt the Event name.
- **Fix:** read the movetext as one token stream, and end a game at its result token.

**P8 · Medium · P2 — One bad game aborts a database import, and the import is left
partly committed.**
- **Code:** `uGameDatabase.pas:572-580`, 829-831; batches are committed every 10,000 games
  (454-460).
- **Examples:** a 3-game file with 1 bad game imports 0 games. 10,001 good games plus 1
  bad one leaves 10,000.
- **Fix:** skip and log bad games, with a savepoint per game.

**P9 · Medium · P2 — The database's continuation summary can go stale silently.**
- **Code:** it is rebuilt only at the end of an import, or on request (825, 851-876), and
  searches use it whenever it is non-empty (1131).
- **Effect:** after a crash mid-import, the move statistics and the game list for a
  position disagree. The position indexes, dropped at the start of the import, stay missing
  until the next open.
- **Fix:** store a freshness stamp, or update the summary in the same transaction.

**P10 · Medium · P2 — A database import runs on the GUI thread while the GUI stays
usable.**
- **Code:** `ProcessMessages` in the progress callback, with a non-modal progress window.
- **Effect:**
  - Browsing triggers position searches without the dropped indexes.
  - Every query commits the import transaction mid-batch.
  - Closing the main window frees the database form while the import is still on the call
    stack (crash not verified).
- **Fix:** a worker thread with its own connection, or a modal dialog with a busy guard.

**P15 · Medium · P2 — A PDN save and load is not an identity.**
- **Losses:**
  - Engine, annotator and Auto Play PVs are written as ordinary `( … )` variations, and
    come back as variations.
  - An annotator score is written on two moves (`PreviousPositionAnnotation`).
  - Status, time control and clocks are not written as tags, and there is no `SetUp`
    tag with the FEN.
- **Together with:** G1, G5, P1-P4. Load then save then load currently loses or changes
  data in several ways.
- **Fix:** mark PVs in a comment (or a PV-only RAV marker) so they reload as PVs. Add a
  round-trip test that compares trees node by node.

**P11 · Medium · P2 — Appending a game to a PDN file reads and rewrites the whole file.**
- **Code:** `AppendPdnTextToFile` (`uPdn.pas:1146-1165`).
- **Effect:** it isn't atomic, so a crash or full disk can truncate a whole tournament
  file. The cost grows with the file. It also turns CRLF into LF.
- **Fix:** open the file in append mode and write only the new game.

**T3 · Medium · P2 — In a single round robin, engine i is White against every j > i.**
- **Code:** `uTournamentPairing.pas:304-318`, and every game is in "round 1".
- **Example:** with 5 engines, the White counts are 4/3/2/1/0.
- **Fix:** Berger tables with alternating colours and real rounds.

**T4 · Medium · P2 — Swiss pairing is greedy, and then claims a repeat was unavoidable.**
- **Code:** `uTournamentPairing.pas:391-410`, 473-486; message at
  `uTournamentDialog.pas:780-785`.
- **Example:** round 3 paired E1-E4 and E5-E2 (a repeat), although E1-E5 plus E2-E4 were
  both new.
- **Fix:** backtracking or matching within score groups.

**T5 · Medium · P2 — The tournament PDN path is unstable, and a write error stalls the
tournament.**
- **Code:** the file name is the tournament name plus the JSON's folder (622-635).
  Before the JSON is first saved, that is a path relative to the working directory, which
  is how `Tournament.pdn` ended up in the repo root.
- **Effects:**
  - Renaming the tournament mid-run splits the games across files.
  - An exception while appending (1597) happens before `Active := False`, so it repeats
    every 250 ms and no new game starts.
- **Fix:** fix the path when the tournament starts and store it in the JSON. Catch write
  errors and mark the row.

**T6 · Medium · P2 — "Create pairing" during a running round robin wipes the results of
running games.**
- **Code:** `CreatePairingClick` (739-799) doesn't check for running games.
  `ClearPairingsClick` and `LoadClick` check only `FRunning`, not games that are still
  stopping.
- **Fix:** disable these actions while `IsRunning` is true.

**T7 · Medium · P2 — A failed tournament JSON load leaves `FLoadingTournament = True`
permanently.**
- **Code:** it is set at 1162, outside the try/finally that resets it.
- **Effects:** after a corrupt file, settings changes no longer invalidate results. A
  half-loaded grid keeps the old file name, so the next Save overwrites the previous
  tournament.
- **Fix:** move the flag inside the try/finally, and load into temporaries first.

**T8 · Medium · P2 — Tournament rows identify engines by display name, including their
slot number.**
- **Code:** names like "Scan (Engine 2 hub engine)" (`uEngineRegistry.pas:317-335`).
- **Effects:**
  - Editing engines.json remaps a saved tournament's rows to a different engine.
  - If no engine matches, the row is never startable but is still returned as next, so
    the tournament "runs" forever with nothing playing.
- **Fix:** store `EngineIdentityKey`, and report rows whose engine is missing.

**D5 · Medium · P2 — The DXP reader treats EOF and engine exit as "no data yet".**
- **Code:** `ReadDxpPacket` (`uDxpEngine.pas:1101-1135`).
- **Effect:** a dead engine is noticed only after D3's 60 s.
- **Fix:** fail at once on EOF, on a hard socket error, or when the process has exited.

**D6 · Medium · P2 — DXP GAMEREQ minutes use banker's rounding.**
- **Code:** `uDxpProtocol.pas:151`.
- **Example:** 1.5 min is sent as 002 and 0.5 min as 001, so the engine can overspend and
  then lose on the GUI's clock.
- **Fix:** `Trunc`, with a minimum of 1.

**D7 · Medium · P2 — Hub `level` never sends the increment, and sends `moves=0` for
sudden death.**
- **Code:** `uHubEngine.pas:995-1013`; `uGameOrchestrator.pas:399-409`, 522.
- **Effect:** protocol.txt says to omit `moves` for sudden death, so engines budget
  Fischer games as if there were no increment.

**D8 · Medium · P2 — The DXP listener binds to 0.0.0.0 and accepts the first peer.**
- **Code:** `uDxpEngine.pas:483`, `uDxpTestConnection.pas:105`.
- **Effect:** any host on the LAN can take the engine's slot.
- **Fix:** bind to 127.0.0.1 (or `DxpHost`), and check the remote address.

**D9 · Medium · P2 — Engine handshake timeouts are short.**
- **Code:** Hub waits 1 s for `id`, 2 s for `wait` and 5 s for `ready`
  (`uHubEngine.pas:781-792`); DXP waits 10 s for GAMEACC.
- **Effect:** engines that load books or bitbases at start-up can fail to launch (not
  verified with a real engine).
- **Fix:** configurable per engine, with 30-60 s defaults.

**F1 · Medium · P2 — Engine logs grow without bound, and are scanned every 25 ms.**
- **Code:**
  - `FStateLog` keeps every line, including all stdout (`uEngineBase.pas:196`).
  - Hub `FReceivedLines` keeps every `info` line (`uHubEngine.pas:369`), and
    `TakeReceivedLine` scans it under the lock on every poll.
  - DXP `FReceivedLines` is filled and never read.
- **Effect:** a long annotator session grows memory and slows every stop and think.
- **Fix:** don't store `info` lines; keep the state log as a ring buffer.

### Low

**A4 · Low · P2 — Nothing shows that the PV browser is frozen or that a newer PV is
pending.**
- **Code:** any click locks it (`uPvBrowser.pas:296-311`), and only Escape inside the memo
  unlocks it. The label keeps showing the old score, depth and time.
- **Fix:** show "browsing — Esc to resume, newer PV pending".

**C1 · Low · P2 — The tests miss the paths where the bugs are, and the mutation audit is
easy to bypass.**
- **Audit:** `source_audit_check.sh` greps for 19 method names, but misses
  `RecordVariationMove`, `RecordVariationAnnotation`, `TrimVariationLine`,
  `SetVariationLineOrigin`, `RecordAutoPlayPVForNodeId`, `SetStatusText`,
  `SetTimeControl` and the `RecordVariation*PV` methods. All `TGameTree` mutators are
  public, and the actor exposes the tree.
- **Tests:**
  - They build trees the way the main form doesn't (G1).
  - There is no test of `uMainForm` paths, the tournament units, several kings in a FEN
    (R1), Black-to-move starts, or standard PDN variations.
  - One test asserts that comments are dropped (P2).
- **Fix:** make the mutators visible only to the actor, for example in a separate unit
  section. Add a headless `TMainForm` test like the probe used here.

**A5 · Low · P3 — The evaluation bar shows an old engine score as if it were current, and
prefers it over a current annotator score.**
- **Code:** `uScoreHistoryControl.pas:452-456`, 772-806.
- **Status:** by code reading (a design decision).

**A6 · Low · P3 — A free-text comment can be plotted as an engine score.**
- **Code:** `uScoreHistoryControl.pas:742-747` treats a whole line as `score=`.
- **Example:** `{+2 for white}` plots as +2.

**A7 · Low · P3 — Two unlocked uses of objects across threads in the DXP code.**
- **Code:** the DXP accept thread's `FServer` is used across threads without a lock
  (`uDxpEngine.pas:461-467` against 504). The test connection frees `FSocket` from the
  calling thread.
- **Status:** a possible race, not reproduced.

**D10 · Low · P3 — The DXP engine's view of the game is inferred, not reconciled with the
game record.**
- **Code:** `DoStartThinking` sends only the last move whenever the count grew, and
  `StartThinking` appends the engine's own move before the GUI validates it.
- **Effect:** correct today, but any takeback or "continue from here" feature would desync
  the engine silently.
- **Fix:** keep the exact list of moves exchanged, and restart the game (GAMEEND plus a
  new GAMEREQ) on any mismatch.

**D11 · Low · P3 — Engine child processes inherit each other's pipes.**
- **Code:** set FD_CLOEXEC on the parent side on Unix. On Windows, the inheritable handles
  in `uPlatformProcess.pas:193-221` can leak between concurrent launches (not verified).
- **Effect:** after a GUI crash, an engine that quits on stdin EOF can stay alive.

**D12 · Low · P3 — Smaller protocol issues.**
- **DXP:**
  - A half-received packet is lost at a timeout, and the 127-byte maximum isn't enforced.
  - MOVE parsing accepts trailing garbage and a capture count of 00. That is harmless only
    because the orchestrator validates moves.
  - GAMEEND is sent while the engine is to move.
  - The Fischer field truncates digits instead of clamping.
- **Hub:**
  - `HubQuote` and `HubParamQuote` are inconsistent.
  - `ExtractHubArgument` can match `name=` inside a quoted value.
  - `CommandLineQuote` mishandles trailing backslashes on Windows.
- **Timing:** the orchestrator times moves with `Now`, so a DST change or clock step
  during a think charges ±1 h. Use `GetTickCount64`.
- **Analyzer:** an exception in `HandleCommand` ends the analyzer thread without any
  message.

**U4 · Low · P3 — Setup position accepts impossible positions.**
- **Examples:** a white man on row 1, an empty side, the same square for both colours.
  The side-to-move setter (`uDraughtsBoard.pas:70`) doesn't regenerate legal moves.

**R6 · Low · P3 — FEN and move-text parsing edge cases.**
- **FEN:** `W:W31 - 35:B1` and a trailing `.` raise.
- **Moves:**
  - `"3 2-28"` is read as `32-28`.
  - `26x17x` is accepted.
  - Very long numbers overflow (harmless).

**R7 · Low · P3 — The FMJD 16-move and 5-move endgame draw rules are missing.**
- **Code:** only threefold repetition and the 25-move king rule are implemented, and both
  correctly (`uGameOrchestrator.pas:361-382`).
- **Example:** three pieces including a king against a lone king runs on to the 25-move
  rule or the clock.

**T9 · Low · P3 — Tournament games that end `*` (error, no move, stopped) are never
replayed automatically.**
- **Code:** a non-empty reason blocks the restart (950, 878).
- **Effect:** the tournament "completes" with games unplayed; in Swiss it blocks the next
  round.

**P12 · Low · P3 — Smaller PDN issues.**
- **Reader:** the database import reader splits only on LF, so a CR-only file gives 2
  games in Open PDN but 1 garbage game in the database.
- **Legacy writer:** it misnumbers Black-to-move starts (`1... 18-23 1. 32-28`) and
  doesn't escape `}` in comments.
- **Results:** both writers export 1-0, 1/2-1/2 and 0-1 instead of FMJD 2-0, 1-1 and 0-2.
  That is a design decision for you.

**P13 · Low · P3 — The database stores no variations and no non-score comments; its
schema has no version.**
- **Code:** no variation column. `LoadGameById` returns none, and the duplicate hash
  ignores annotations. "Create database" on an existing file appends to it.

**P14 · Low · P3 — Name filters are not case-insensitive for non-ASCII names.**
- **Code:** SQLite `lower()` folds only ASCII.
- **Examples:** "Émile" matches but "émile" doesn't. `%` and `_` act as wildcards.

**F2 · Low · P3 — Open PDN parses the whole file on the GUI thread.**
- **Example:** 136 MB (11,600 games) took 24 s, with no progress or cancel.
- **Fix:** read headers first, and parse a game only when it is selected.

**F3 · Low · P3 — Every database import rebuilds the indexes and summary over the whole
table.**
- **Code:** `uGameDatabase.pas:819-827`, 861-869.
- **Effect:** importing 10 games into a large database costs a full rebuild.

**F4 · Low · P3 — One display refresh replays the game from the start about 10 times.**
- **Code:** `TryBuildDisplayedBoard` and `TryBuildTreeBoardAtNode` are called separately by
  the board, highlights, legal moves, analyzer, MCTS, PV panes and database search. Each
  replays from the starting FEN with full move generation.
- **Effect:** negligible for normal games, but cheap to fix with one cached board per
  refresh. It goes away with G7's `BoardAtNode`.

**C2 · Low · P3 — Dead code that has drifted from the live code.**
- **Unused:** `FindNextUnplayedTournamentRow` (it differs from the dialog's own version),
  `TGameMoveHistory.AssignFromTexts` (it always assumes White starts),
  `StripPdnMoveText` and `ExtractMovesFromPdnText`.
- **Hub:** the `ReceiveMessage` path, including a `go thiink` alias.
- **Engine parameter:** `gui-single-captures-include-captured-square` is never read.

**C3 · Low · P3 — Repository hygiene.**
- **Tracked files:**
  - `prefs.json` is machine-specific, and is rewritten next to the executable whenever
    preferences change, so the working tree goes dirty.
  - `Tournament.pdn` is runtime output.
  - The generated `output/pdf` manual.
- **Untracked and ignored:** `.o`/`.ppu` files in the root and `tests/`, `tmp_db_smoke.o`,
  and `GWD/` with engine binaries.
- **Build:** there is no build script. The tests need `fpc` and `rg` (ripgrep) on PATH.
  Config files next to the executable fail in a read-only install folder.

### Notes (no issue IDs)

- **Actor queue:** all commands are executed on the GUI thread, so the actor's queue
  (`PostCommand` and `ProcessCommands`) is used only for `ReleaseSearch`. Synchronous
  execution would be simpler; keep the queue only if engines are ever to post into the
  tree directly.
- **Game tree window:** it is read-only. Clicking a node only freezes updates; it does not
  navigate. With node-ID cursors (G7), making it a navigator is easy.
- **PDN export changes the tree:** `PrepareGameTreeForPdn` writes the header back into
  the tree on every Save or Copy PDN, taking the standalone names from the read-only edit
  boxes. Exporting should not change the tree.
- **Turn checks use parity:** `IsHumanToMove` and `SideToMoveAfterPly` use mainline-count
  parity, not a board. That is fine while the tree has no holes (G6).
- **Other shadows from review 01:**
  - `FAutoPlayMovesText` is a copy of the start node's `AutoPlayPV`, and is appended to
    instead of being read back from the node.
  - `TDraughtsBoard.FMovesPlayed` is filled during every replay but never used.
  - `FVariationBrowseMoves` is a deliberate preview of moves not in the tree. It is fine
    as such, but about 35 code paths branch on it. Representing it as
    `(nodeId, pvMoves, pvIndex)` would make it explicit.
- **Comment handling:** comments are replaced on every rebuild from the inputs
  (`ResetNodeState` clears `CommentText`, and only the inputs restore it). Once nodes own
  their data, a rebuild should not touch comments the user added.

## Issue status at 98da11d

| ID | Issue | Severity | Priority | Status |
|---|---|---|---|---|
| A1 | Annotator stores the previous position's PV/score on the newly browsed node | High | P1 | New |
| D1 | After a slow `stop`, the old search's `done move=` is played for the next search | High | P1 | New |
| D2 | Crashed DXP engine can kill the GUI with SIGPIPE (Unix) | High | P1 | New |
| D3 | Fixed 60 s per-move engine timeout, independent of the time control | High | P1 | New |
| D4 | DXP GAMEEND "I win"/"draw" claims become the official result | High | P1 | New |
| G1 | PDN variations never become tree nodes: invisible, dropped by Save PDN | High | P1 | New |
| G2 | Adding a move in or at a variation truncates the existing line; mainline move stored as duplicate variation | High | P1 | New |
| G3 | Analysis and clocks keyed by ply number / variation index re-attach to different moves | High | P1 | New |
| G4 | Rebuild re-adds the previous game's variations to the new game | High | P1 | New |
| M3 | Moves added right after Stop wiped by the runner's next snapshot | High | P1 | New |
| M4 | Stop just after game over can replace the result with `*`; stored snapshot pushed back into the tree | High | P1 | New |
| P1 | Standard PDN variations imported from the wrong ply; GUI exports them in a non-standard place | High | P1 | New |
| R1 | `CurrentFEN` drops K from the second and later kings | High | P1 | New |
| T1 | Tournament standings only in memory: no autosave, no link to PDN records | High | P1 | New |
| T2 | Tournament grid results can disagree with the game records | High | P1 | New |
| U1 | Unsaved standalone (PDN/FEN) game becomes unreachable or is wiped when an engine game is started, selected or removed | High | P1 | New |
| U2 | Main board replays without validation: short-form multi-capture raises; board can differ from other views | High | P1 | New |
| P2 | Free-text PDN comments discarded on import | High | P2 | New |
| A3 | Frozen PV browser's mini board overwritten with the game position | Medium | P1 | New |
| G5 | Root variations never rendered or exported; standalone mainline can't be extended | Medium | P1 | New |
| R2 | `PlayMove` mutates the board before validating; no rollback | Medium | P1 | New |
| A2 | PV browser silently shows cut-off or impossible boards | Medium | P2 | New |
| D5 | DXP reader treats EOF/engine exit as "no data yet" | Medium | P2 | New |
| D6 | DXP GAMEREQ minutes use banker's rounding | Medium | P2 | New |
| D7 | Hub `level` omits the increment and sends `moves=0` for sudden death | Medium | P2 | New |
| D8 | DXP listener binds to 0.0.0.0 and accepts the first peer | Medium | P2 | New |
| D9 | Short Hub/DXP handshake timeouts | Medium | P2 | New |
| F1 | Unbounded engine log/line buffers, scanned every 25 ms | Medium | P2 | New |
| G6 | Tree accepts any text as a move, at any ply (holes); PDN variations unvalidated | Medium | P2 | New |
| G7 | Game tree not structurally a tree: mainline under root, parent meaning varies, index in variation key | Medium | P2 | New |
| G8 | Score-annotation projection fills gaps forward and is fed back into the tree | Medium | P2 | New |
| M1 | Per-move engine PV and time stay in the orchestrator; tree gets a lossy sample | Medium | P2 | New |
| M2 | Illegal human move ends the game with an error instead of being rejected | Medium | P2 | New |
| P3 | Nested PDN variations dropped on import | Medium | P2 | New |
| P4 | Multi-line PDN comment shifts annotations to the wrong moves | Medium | P2 | New |
| P5 | Result token in the movetext ignored | Medium | P2 | New |
| P6 | `0-0` parsed as a move; aborts database import | Medium | P2 | New |
| P7 | PDN game boundaries depend on `[` at line start | Medium | P2 | New |
| P8 | One bad game aborts a database import, partly committed | Medium | P2 | New |
| P9 | Database continuation summary can go stale silently | Medium | P2 | New |
| P10 | Database import on the GUI thread with the GUI usable | Medium | P2 | New |
| P11 | PDN append rewrites the whole file, non-atomically | Medium | P2 | New |
| P15 | PDN save/load not an identity (PVs as variations, scores twice, no SetUp/time control) | Medium | P2 | New |
| R3 | Unvalidated `PlayMove` accepts wrong-side and impossible moves | Medium | P2 | New |
| R4 | `LoadFromFEN` sets the side to move before parsing succeeds | Medium | P2 | New |
| R5 | Landing-square capture paths rejected; export uses captured-square lists | Medium | P2 | New |
| T3 | Round robin: engine i always White against j > i; all games in round 1 | Medium | P2 | New |
| T4 | Swiss pairing greedy; false "repeat unavoidable" message | Medium | P2 | New |
| T5 | Tournament PDN path unstable; write error stalls the tournament | Medium | P2 | New |
| T6 | "Create pairing" during a running round robin wipes running results | Medium | P2 | New |
| T7 | Failed tournament JSON load leaves `FLoadingTournament` set | Medium | P2 | New |
| T8 | Tournament rows identify engines by display name with slot number | Medium | P2 | New |
| U3 | Move at the end of a finished engine game extends the official mainline past the result | Medium | P2 | New |
| A4 | Frozen PV browser and pending PV not indicated | Low | P2 | New |
| C1 | Tests miss the main-form paths; mutation audit incomplete and bypassable | Low | P2 | New |
| A5 | Evaluation bar shows an old engine score as current | Low | P3 | New |
| A6 | Free-text comment can be plotted as an engine score | Low | P3 | New |
| A7 | DXP accept thread / test connection share sockets without locks (unverified race) | Low | P3 | New |
| C2 | Dead code drifted from live code | Low | P3 | New |
| C3 | Repository hygiene: tracked prefs/runtime output, config next to the executable | Low | P3 | New |
| D10 | DXP engine's view of the game inferred, never reconciled | Low | P3 | New |
| D11 | Engine child processes inherit each other's pipes | Low | P3 | New |
| D12 | Smaller DXP/Hub protocol issues; move timing by wall clock | Low | P3 | New |
| F2 | Open PDN parses the whole file on the GUI thread | Low | P3 | New |
| F3 | Database import rebuilds all indexes and the summary | Low | P3 | New |
| F4 | About 10 full replays per display refresh | Low | P3 | New |
| P12 | Smaller PDN reader/writer issues; result export format | Low | P3 | New |
| P13 | Database stores no variations or comments; no schema version | Low | P3 | New |
| P14 | Name filters not case-insensitive for non-ASCII | Low | P3 | New |
| R6 | FEN and move-text parsing edge cases | Low | P3 | New |
| R7 | FMJD 16-move and 5-move draw rules missing | Low | P3 | New |
| T9 | Tournament games ending `*` never replayed | Low | P3 | New |
| U4 | Setup position accepts impossible positions | Low | P3 | New |