SP0.12: A3 determinism re-validation + vyhybna integration gate for the lifted seam (#747)

* SP0.12: add conflict-event assertion to A3 gate + VyhybnaLiftedDriverIntegrationTest

* SP0.12: fix VyhybnaLiftedDriverIntegrationTest to use lock-step handshake

* SP0.12: address code review - fix comment style and simplify conflict events list

* SP0.12: rename Vyhybna -> ShuntingLoop in test class/method names (keep vyhybna.xml)

* Fix 6 findings from Goal 10 code review (safety gate, tearing, dedup)

Addresses correctness bugs and a test-coverage gap surfaced by a full-branch
code review of the Goal 10 SP0.x dispatcher-agent series:

- Train.setTargetSpeed's "cleared path ahead" gate checked any block
  ownership instead of a reserved-but-not-occupied block, making it
  effectively non-functional once a train started moving. Added
  PathReservationService.getOccupiedBlocks() and tightened the check.
- AgentLoopDriver read three separate ShuntingLoop observation fields per
  cycle, which could tear across sim ticks. Collapsed to a single
  getLatestObservation() call.
- ShuntingLoopSmokeTest and TrainMovementIntegrationTest never wired
  wireSynchronousDispatcher, so their admission/movement assertions passed
  vacuously. Wired the dispatcher and replaced loose assertions with real
  admission/exit/capacity checks (manually verified they now fail without
  the wiring).
- DefaultNetworkActuatorPort.setSwitchPosition checked lock state before
  the already-in-position check, violating its own idempotent-success
  contract for a locked switch already in the requested position.
- DispatchDecisionApplier's duplicate-reservation guard never evicted
  entries, which could permanently block a legitimate re-reservation after
  a train reversal. Added evictReservationsFor(), wired to BLOCK_RELEASED.
- DefaultNetworkPerceptionPort/DefaultNetworkActuatorPort each hand-rolled
  the same grid-scan loop. Extracted RailwayNetGrid.cellsOfType().

Also documents RuleBasedDispatcher.DEFAULT_MAX_CONCURRENT_TRAINS /
ShuntingLoop.getMaxConcurrentTrains() as a station-topology safety
invariant (not a tunable knob) and asserts it across the touched tests.

Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com>
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Author: 3 people

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/context/navigation/DefaultPathReservationService.kt

@@ -869,6 +869,11 @@ class DefaultPathReservationService(
 	 */
 	override fun getReservedBlocks(trainId: String): List<DynamicTrackBlock> = registry.getBlocks(trainId)
 
+	/**
+	 * Get all blocks currently physically occupied by a train.
+	 */
+	override fun getOccupiedBlocks(trainId: String): List<DynamicTrackBlock> = registry.getOccupiedBlocks(trainId)
+
 	/**
 	 * Implementation of reservePathToAny with intelligent target prioritization.
 	 *

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/context/navigation/PathReservationService.kt

@@ -242,6 +242,18 @@ interface PathReservationService {
 	 */
 	fun getReservedBlocks(trainId: String): List<DynamicTrackBlock>
 
+	/**
+	 * Get all blocks currently physically occupied by a train.
+	 *
+	 * This is a subset of [getReservedBlocks]: every occupied block is also reserved for
+	 * the occupying train, but a train may hold additional blocks that are reserved-but-not-yet-occupied
+	 * (the cleared path ahead of it).
+	 *
+	 * @param trainId Unique identifier for the train
+	 * @return List of blocks currently occupied by this train (empty if none)
+	 */
+	fun getOccupiedBlocks(trainId: String): List<DynamicTrackBlock>
+
 	/**
 	 * Find and reserve path from separator to any next semaphore via specific track section.
 	 *

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/ports/DefaultNetworkActuatorPort.kt

@@ -16,8 +16,8 @@ import cz.vutbr.fit.interlockSim.objects.cells.DynamicRailSemaphore
 import cz.vutbr.fit.interlockSim.objects.cells.DynamicRailSwitch
 import cz.vutbr.fit.interlockSim.objects.cells.RailSwitch
 import cz.vutbr.fit.interlockSim.objects.cells.Signal
-import cz.vutbr.fit.interlockSim.objects.core.Cell
 import cz.vutbr.fit.interlockSim.objects.core.DynamicPathSeparator
+import cz.vutbr.fit.interlockSim.util.cellsOfType
 import io.github.oshai.kotlinlogging.KotlinLogging
 
 private val logger = KotlinLogging.logger {}
@@ -141,12 +141,12 @@ class DefaultNetworkActuatorPort(
 				logger.debug { "setSwitchPosition: unknown switch '$switchName'" }
 				return false
 			}
+		if (sw.conf == position) return true
 		if (sw.locked) {
 			// Switch is locked by a route reservation — cannot be moved while a path is active.
 			logger.debug { "setSwitchPosition: switch '$switchName' is locked by route reservation" }
 			return false
 		}
-		if (sw.conf == position) return true
 		return try {
 			sw.changeConf()
 			sw.conf == position
@@ -197,34 +197,20 @@ class DefaultNetworkActuatorPort(
 	 * Scans the grid once to build a name→semaphore index.  Mirrors the strategy in
 	 * [DefaultNetworkPerceptionPort.buildSemaphoreCache].
 	 */
-	private fun buildSemaphoreCache(): Map<String, DynamicRailSemaphore> {
-		val grid = env.getRailWayNetGrid()
-		val result = mutableMapOf<String, DynamicRailSemaphore>()
-		for (x in 0 until grid.cols) {
-			for (y in 0 until grid.rows) {
-				val cell: Cell? = grid.getCellAt(x, y)
-				if (cell is DynamicRailSemaphore && cell.name.isNotBlank()) {
-					result[cell.name] = cell
-				}
-			}
-		}
-		return result
-	}
+	private fun buildSemaphoreCache(): Map<String, DynamicRailSemaphore> =
+		env
+			.getRailWayNetGrid()
+			.cellsOfType<DynamicRailSemaphore>()
+			.filter { it.name.isNotBlank() }
+			.associateBy { it.name }
 
 	/**
 	 * Scans the grid once to build a name→switch index.
 	 */
-	private fun buildSwitchCache(): Map<String, DynamicRailSwitch> {
-		val grid = env.getRailWayNetGrid()
-		val result = mutableMapOf<String, DynamicRailSwitch>()
-		for (x in 0 until grid.cols) {
-			for (y in 0 until grid.rows) {
-				val cell: Cell? = grid.getCellAt(x, y)
-				if (cell is DynamicRailSwitch && cell.name.isNotBlank()) {
-					result[cell.name] = cell
-				}
-			}
-		}
-		return result
-	}
+	private fun buildSwitchCache(): Map<String, DynamicRailSwitch> =
+		env
+			.getRailWayNetGrid()
+			.cellsOfType<DynamicRailSwitch>()
+			.filter { it.name.isNotBlank() }
+			.associateBy { it.name }
 }

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/ports/DefaultNetworkPerceptionPort.kt

@@ -18,6 +18,7 @@ import cz.vutbr.fit.interlockSim.objects.core.TrackFacility
 import cz.vutbr.fit.interlockSim.objects.tracks.DynamicTrackBlock
 import cz.vutbr.fit.interlockSim.sim.Train
 import cz.vutbr.fit.interlockSim.util.DynamicWrapperUtils
+import cz.vutbr.fit.interlockSim.util.cellsOfType
 
 /**
  * Simulation-backed implementation of [NetworkPerceptionPort].
@@ -298,17 +299,5 @@ class DefaultNetworkPerceptionPort(
 	 * Scans the grid once at construction time to build the semaphore list.
 	 * Matches the strategy used by the animation controller.
 	 */
-	private fun buildSemaphoreCache(): List<DynamicRailSemaphore> {
-		val grid = env.getRailWayNetGrid()
-		val result = mutableListOf<DynamicRailSemaphore>()
-		for (x in 0 until grid.cols) {
-			for (y in 0 until grid.rows) {
-				val cell = grid.getCellAt(x, y)
-				if (cell is DynamicRailSemaphore) {
-					result.add(cell)
-				}
-			}
-		}
-		return result.toList()
-	}
+	private fun buildSemaphoreCache(): List<DynamicRailSemaphore> = env.getRailWayNetGrid().cellsOfType()
 }

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/RuleBasedDispatcher.kt

@@ -85,7 +85,18 @@ class RuleBasedDispatcher(
 	val maxConcurrentTrains: Int = DEFAULT_MAX_CONCURRENT_TRAINS
 ) : Dispatcher {
 	companion object {
-		/** Physical capacity of the vyhybna shunting loop: k1 and k2. */
+		/**
+		 * Physical capacity of the `vyhybna.xml` shunting loop station: 2 parallel
+		 * tracks (k1, k2).
+		 *
+		 * This is a station-topology constant, not a tunable performance knob: a real
+		 * interlocking's concurrent-train capacity is determined by railway-civil-engineering
+		 * expertise at station design time (how many parallel tracks/platforms a given
+		 * station has), the same way [maxConcurrentTrains] is fixed per [RuleBasedDispatcher]
+		 * instance rather than adjusted at runtime. Changing this default only makes sense
+		 * if the bundled `vyhybna.xml` topology itself changes; a different station layout
+		 * should pass its own capacity via the [maxConcurrentTrains] constructor parameter.
+		 */
 		const val DEFAULT_MAX_CONCURRENT_TRAINS: Int = 2
 		private val logger = KotlinLogging.logger {}
 	}

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/ShuntingLoop.kt

@@ -198,10 +198,15 @@ class ShuntingLoop(
 	 * [latestObservation] reference write, so a reader either sees all three from tick N
 	 * or all three from tick N-1 — never a mix.
 	 *
+	 * Public so off-kernel callers (e.g. [cz.vutbr.fit.interlockSim.dispatcher.AgentLoopDriver])
+	 * can read all three fields with a single [getLatestObservation] call instead of three
+	 * separate accessor calls, which would defeat the single-tick atomicity this type exists
+	 * to guarantee.
+	 *
 	 * @since Issue #733 (SP0.11 — Goal 10); single-snapshot publication added by the
-	 *   SP0.11 review follow-up
+	 *   SP0.11 review follow-up; made public by the tearing-fix follow-up
 	 */
-	private data class TickObservation(
+	data class TickObservation(
 		val queuedTrains: List<QueuedTrainObservation>,
 		val innerBlockInputs: List<BlockInputObservation>,
 		val outerBlockInputs: List<BlockInputObservation>
@@ -441,6 +446,21 @@ class ShuntingLoop(
 	 */
 	fun getOuterBlockInputs(): List<BlockInputObservation> = latestObservation.outerBlockInputs
 
+	/**
+	 * Returns the full per-tick observation bundle (queued trains plus inner/outer block
+	 * inputs) as published at the start of the most recent [iteration], in a single
+	 * @Volatile read. Safe to call from off-kernel threads.
+	 *
+	 * Prefer this over calling [getQueuedTrains]/[getInnerBlockInputs]/[getOuterBlockInputs]
+	 * separately when a caller needs more than one of the three fields together — three
+	 * independent calls can observe different sim ticks if the sim thread republishes
+	 * [latestObservation] in between, tearing the single-tick guarantee [TickObservation]
+	 * exists to provide.
+	 *
+	 * @since Issue #733 (SP0.11 — Goal 10); added by the tearing-fix follow-up
+	 */
+	fun getLatestObservation(): TickObservation = latestObservation
+
 	/**
 	 * Returns a snapshot of the currently approved (active) trains. Used by the external
 	 * [DefaultNetworkPerceptionPort][cz.vutbr.fit.interlockSim.ports.DefaultNetworkPerceptionPort]
@@ -539,6 +559,19 @@ class ShuntingLoop(
 
 	fun getTrainsExited(): Int = trainsExitedCount
 
+	/**
+	 * Peak number of trains simultaneously approved (holding an admitted slot) at any
+	 * observed tick during this run.
+	 *
+	 * **Safety invariant:** this value must never exceed the station's fixed topology
+	 * capacity ([RuleBasedDispatcher.DEFAULT_MAX_CONCURRENT_TRAINS], or whatever
+	 * `maxConcurrentTrains` the wired dispatcher was constructed with) — that capacity is
+	 * not a tunable performance knob, it is how many parallel tracks the physical station
+	 * actually has (2 for `vyhybna.xml`: k1, k2). If a dispatcher ever let a 3rd train in
+	 * concurrently on this topology, that train would have no physical track left to occupy —
+	 * an unresolvable state, not a recoverable "wait and retry" condition, since every
+	 * available parallel track is already held by the other two approved trains.
+	 */
 	fun getMaxConcurrentTrains(): Int = maxConcurrentTrainsCount
 
 	fun getBlockTransitions(trainId: String): Int = blockTransitionsByTrain[trainId] ?: 0

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/sim/Train.kt

@@ -1360,9 +1360,11 @@ class Train :
 	fun setTargetSpeed(speed: Double) {
 		require(speed >= 0.0) { "Target speed must be >= 0, got $speed" }
 		if (speed > 0.0) {
-			val reservedBlocks = env.getRoutingServices().getPathReservationService().getReservedBlocks(name)
-			if (reservedBlocks.isEmpty()) {
-				logger.warn { "Train $number: setTargetSpeed($speed) ignored — no route reserved for $name" }
+			val pathReservationService = env.getRoutingServices().getPathReservationService()
+			val reservedBlocks = pathReservationService.getReservedBlocks(name)
+			val occupiedBlocks = pathReservationService.getOccupiedBlocks(name).toSet()
+			if (reservedBlocks.all { it in occupiedBlocks }) {
+				logger.warn { "Train $number: setTargetSpeed($speed) ignored — no cleared block ahead for $name" }
 				return
 			}
 		}

core/src/commonMain/kotlin/cz/vutbr/fit/interlockSim/util/GridScanUtils.kt

@@ -0,0 +1,32 @@
+/* Brno University of Technology
+ * Faculty of Information Technology
+ *
+ * BSc Thesis  2006/2007
+ *
+ * Railway Interlocking Simulator
+ *
+ * Bedrich Hovorka
+ */
+package cz.vutbr.fit.interlockSim.util
+
+import cz.vutbr.fit.interlockSim.context.RailwayNetGrid
+import cz.vutbr.fit.interlockSim.objects.core.Cell
+
+/**
+ * Returns every cell of type [R] currently placed in this grid, scanning each `x`/`y`
+ * coordinate via [RailwayNetGrid.getCellAt].
+ *
+ * @since Goal 10 code-review fix — replaces the near-identical grid-scan loops previously
+ *   duplicated across [cz.vutbr.fit.interlockSim.ports.DefaultNetworkPerceptionPort] and
+ *   [cz.vutbr.fit.interlockSim.ports.DefaultNetworkActuatorPort]
+ */
+inline fun <reified R : Cell> RailwayNetGrid<Cell>.cellsOfType(): List<R> {
+	val result = mutableListOf<R>()
+	for (x in 0 until cols) {
+		for (y in 0 until rows) {
+			val cell = getCellAt(x, y)
+			if (cell is R) result.add(cell)
+		}
+	}
+	return result
+}

core/src/jvmTest/kotlin/cz/vutbr/fit/interlockSim/ports/DefaultNetworkActuatorPortTest.kt

@@ -388,6 +388,14 @@ class DefaultNetworkActuatorPortTest {
 			val p = port(cells = mapOf((0 to 0) to sw))
 			assertThat(p.setSwitchPosition("vA", RailSwitch.Conf.MAIN)).isTrue()
 		}
+
+		@Test
+		@DisplayName("returns true when switch is locked but already in requested position (idempotent no-op)")
+		fun lockedButAlreadyInPositionReturnsTrue() {
+			val sw = switch("vA", conf = RailSwitch.Conf.MAIN, locked = true)
+			val p = port(cells = mapOf((0 to 0) to sw))
+			assertThat(p.setSwitchPosition("vA", RailSwitch.Conf.MAIN)).isTrue()
+		}
 	}
 
 	// ── setSignalAspect ─────────────────────────────────────────────────────