Goal 10 parent PR#721
Draft
bedaHovorka wants to merge 16 commits into
Draft
Conversation
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
bedaHovorka
force-pushed
the
goal-10
branch
2 times, most recently
from
5df3642 to
03f884f
Compare
8 tasks
* Add operating vocabulary DSL design in .md
* Add SP3.1 LLM model evaluation doc for Goal 10 agents * Finalize SP3.1 doc per rewritten #532 canonical description .md file
…tcher-agent module (#720) * SP0.1: Extract ShuntingLoop dispatch to RuleBasedDispatcher; add :dispatcher-agent module * desktop-ui module after agent to settings.gradle.kts * SP0.1 review fixes: split Dispatcher seam, widen tick context, add unit tests Address PR #720 code-review findings (Important #1-#4, Minor #6-#7): - #1 Restore original approve -> hold -> check ordering: split Dispatcher.tick() into approve() (before polling hold) and advancePaths() (after it) so ShuntingLoop can place hold(1.0) between admission and path-advancement, matching the d8780c7 baseline. Re-add the "Train entry events align with polling" comment. - #2 Remove FIFO bias from the seam: replace pollUnapproved() + unapprovedTrainCount with unapprovedTrains: List<Train> so a future LLM dispatcher can select by priority, not just pop head. approveTrain(train) now takes the specific train; the shell removes that exact train from the queue. - #7 Drop leaky toDynamic callback: replace with high-level isPathSetUp(block, to): Boolean. The getSecondEnd + env.toDynamic + block.isSetUpPath conversion stays inside the shell; dispatchers reason in blocks and semaphores, not internal wrappers. - #3 Add branch-level unit tests for RuleBasedDispatcher (core/jvmTest) with a fake DispatcherTickContext + MockK stubs covering approve cap/empty, all checkOneEnd branches, checkBothEnds short-circuit, and the constructor guard. - #4 Fix RuleBasedDispatcherDeterminismTest docstring: per-train transition count is 2 (verified: [2,2,2,2,2]), not 7. - #6 Correct PR #720 module graph: :core <- :dispatcher-agent at SP0.1; :fast-sim depends only on :core; the :dispatcher-agent <- :desktop-ui edge is planned for Stage B. Verification: ./gradlew build green (core/desktop-ui/dispatcher-agent detekt+ktlint+test+integrationTest, fast-sim native compile).
Define NetworkPerceptionPort with read-only sensor readings (block occupancy, semaphore state, train position, timetable) so a future LLM dispatcher can observe network state without depending on mutable domain internals. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* Changes before error encountered Agent-Logs-Url: https://github.com/bedaHovorka/interlockSim/sessions/eb261862-33e9-46b3-a285-160ebfd5c5b7 * SP0.3: fix ktlint violations in ActuatorPortsTest * SP0.3: apply actuator-port review fixes (#723) Address code-review findings on PR #723 (Important #1-#5, Minor #1-#3): - Move TrainActuatorPort / NetworkActuatorPort / ActuatorPortsTest from cz.vutbr.fit.interlockSim.sim to cz.vutbr.fit.interlockSim.ports so the act-side ports sit beside their #722 sense-side siblings. Test stays in commonTest so it still runs on linuxX64 for :fast-sim. - Add NetworkActuatorPort.releaseRoute(trainName): Boolean — the symmetric counterpart to requestRoute (idempotent; throws on blank name). Mirrors PathReservationService.releasePath so the "stable" seam ships with a release command before SP1.4 binds real impls. - Tighten setSignalAspect contract: false also covers interlocking safety refusal (no compatible reserved route, conflicting switch state), aligning with setSwitchPosition's two failure modes and the class-level guarantee. - Specify requestRoute invalid-input: blank trainName and unknown InOut names throw IllegalArgumentException (unknown names must fail fast); NoRouteExists is reserved for valid-but-disconnected endpoints. - Document AllPathsBlocked.attemptedPaths >= 0. - Tests: add ContractValidatingTrainActuator (negative-speed throw) and ContractValidatingNetworkActuator (blank/unknown-name throws) stubs with assertFailsWith tests; add releaseRoute forwarding + return tests; rewrite exhaustive-when test with real per-case assertions (compile-time exhaustiveness preserved via else-less when); replace redundant as cast with assertk reified isInstanceOf<T>() chaining. Verification: :core:detekt, :core:ktlintCheck, :core:checkCoreCommonMainPurity green; ActuatorPortsTest 21/21 on JVM and linuxX64; ./gradlew build green.
* SP0.4: observable simulation state at a control step (#543) * SP0.4 follow-up: narrow snapshot simTime fallback + add frozen-snapshot test Address review Minor #2 and #3 from PR #725 (#543): - DefaultNetworkPerceptionPort.snapshot(): replace broad runCatching { Process.time() }.getOrDefault(0.0) with a try/catch narrowed to DiscoException, so the "not inside a kDisco simulation" fallback still returns 0.0 but genuine unexpected exceptions (NPE, etc.) propagate instead of being silently masked as simTime = 0.0. - Add snapshotIsFrozenFromLaterSourceMutation test: captures a snapshot, mutates the underlying mock source state and clears the active-trains list, then asserts the captured readings are unchanged — locking the frozen/isolated-snapshot contract. No sim/ logic, RuleBasedDispatcher, or public API behavior changed.
* SP0.6: Implement DefaultNetworkActuatorPort, DefaultTrainActuatorPort, and wire networkActuator into DispatcherTickContext (#545) * SP0.6: Add safety guards and partial-route support per review feedback * SP0.6: Rename requestRoute parameters from InOut to endpoint names per review * SP0.6: Refine actuator port contract per review (I1/I2/Q1) Code-review corrections for PR #726: * setSignalAspect: remove the isAllowing() refusal guard. Both upgrades and downgrades on a dynamic semaphore are now permitted (downgrades are physically hard for a train to reach due to braking, but allowed at the command surface). Constant semaphores (predzvest/naraznik/rychlostnik) stay constant via a post-condition `return sem.signal == signal`, which also fixes the false-success reported for constant semaphores whose setter is a no-op (I1). KDoc on NetworkActuatorPort.setSignalAspect updated to the new contract. * requestRoute: surface reservation conflicts as a distinct RouteRequestResult.Conflict(blockName, existingOwner) instead of collapsing them to AllPathsBlocked(0), which discarded the owning train and block an LLM dispatcher could reason about (I2). String-only fields honour the port's no-domain-object-leak contract. KDoc + usage example updated; ActuatorPortsTest exhaustiveness guard extended. Q2 (setSwitchPosition sw.locked guard) and Q3 (Train.setTargetSpeed backstop) are left unchanged per owner decision -- Q2 is correct as-is, Q3 deferred to a later SP. Tests: SetSignalAspect tests rewritten to use real DynamicRailSemaphore instances (constant vs dynamic) so the post-condition is exercised end-to-end, not just mocks; Conflict-mapping tests assert the new RouteRequestResult.Conflict carrying block/owner (named and unnamed).
* SP0.8: Add ActuatorCommandQueue with tests * SP0.8: Address review feedback on ActuatorCommandQueue * SP0.8: Document approximateSize race and use getCount() * SP0.8: Use dedicated lock object and clarify postAll KDoc * SP0.8: Expand KDoc and stabilize consumer test * SP0.8: Keep addAll under lock and batch size decrement * SP0.8: Fail-fast capacity check and stable consumer termination * SP0.8: Clarify capacity error and extract test timeout constant * SP0.8: Extract magic numbers and fix error message grammar * SP0.8: Fix approximateSize in unlimited mode, postAll KDoc, and consumer test determinism Track the size counter unconditionally so approximateSize() is correct in unlimited (negative-capacity) mode, not silently 0; gate only the capacity check on capacity > 0 to preserve the lock-free unlimited path. Reword the postAll KDoc to drop the inaccurate 'non-blocking' claim in favor of an accurate description of the capacity-accounting critical section. Replace the consumer stress-test's getCount()+empty-drains termination heuristic with await()+final-drain, whose happens-before guarantee makes the test provably deterministic; remove the now-unused EMPTY_DRAINS_THRESHOLD.
…on (#735) * SP0.7: Move DispatchDecision into :core, add DispatchObservation Additive, zero-behavior-change step for Issue #729: introduces the new DispatchObservation/QueuedTrainObservation/BlockEndObservation types the pure Dispatcher.decide() seam will consume, and relocates DispatchDecision from :dispatcher-agent (SP0.1 placeholder) into :core, since Dispatcher and RuleBasedDispatcher live in :core and :dispatcher-agent depends on :core, never the reverse. Updates ActuatorCommandQueue's import accordingly (its PR landed on goal-10 after this branch was cut). * SP0.7: Reshape Dispatcher to a pure decide() seam Replaces the imperative Dispatcher.approve(context)/advancePaths(context) pair (DispatcherTickContext callbacks) with a single pure decide(observed: DispatchObservation): List<DispatchDecision>. This is the seam both RuleBasedDispatcher and the future LLM dispatcher (SP2b) sit behind. RuleBasedDispatcher now reads DispatchObservation/BlockEndObservation value data instead of live simulation objects, and returns decisions instead of calling callbacks. Both ends of an inner block are evaluated independently (the old checkBothEnds short-circuit depended on a live actuation return value that a pure decide() cannot have; behaviorally equivalent on real data since a block's occupant/reservedFrom are single-valued per block — see RuleBasedDispatcher KDoc). ShuntingLoop remains the single, synchronous, in-kernel caller for this slice: it builds a DispatchObservation each phase and applies the returned decisions inline via new private methods that reproduce exactly what DispatcherTickContext's callbacks did (admitting a train, resolving a semaphore by name and reserving a path through the existing tryReservePathFrom). No ActuatorCommandQueue, no second thread, no :dispatcher-agent involvement here — that lifted, cross-thread driver is SP0.8-SP0.10 (already landed/in flight separately). RuleBasedDispatcherTest is rewritten against DispatchObservation value classes (no more mocks/fake context). RuleBasedDispatcherDeterminismTest needed no source changes; 10 consecutive vyhybna.xml runs still produce identical trainsExited/maxConcurrentTrains/block-transition counts, confirming the refactor is behavior-preserving. * SP0.7: Make ReservePath explicit from→to, rename ends→inputs (#735 follow-up) Address PR #735 review: the dispatcher decision must carry an explicit from→to path, and the seam's vocabulary is "inputs" (a train entering the next section at a block entry point), not ShuntingLoop-specific "ends". DispatchDecision.ReservePath is now (trainId, fromSemaphoreName, toSeparatorName) — one section, `to` = next separator toward the destination (a semaphore, or the destination InOut for the final section). The shell pre-computes `to` as the first FREE next separator and the applier calls PathReservationService.reservePath(train, from, to) directly, reproducing the pre-#729 reservePathToAnyNextSemaphore outcome. Behavior preservation: rather than reimplementing the next-semaphore selection heuristic in the shell (a first attempt broke the 3 ShuntingLoop integration tests — the forward-facing direction() check was wrong), expose a read-only PathReservationService.findNextReservationTarget that reuses the existing findNextSemaphoresVia + isPathAvailable logic — the read-only twin of reservePathToAnyNextSemaphore. The determinism gate (RuleBasedDispatcherDeterminismTest, 10x vyhybna.xml) stays 10/10 green. Rename (type + fields + methods + KDoc + tests): BlockEndObservation -> BlockInputObservation innerBlockEnds/outerBlockEnds -> innerBlockInputs/outerBlockInputs isApproachingThisEnd/pathSetUpTowardThisEnd -> ...ThisInput decideForEnd -> checkInput, decidePathAdvancements -> checkAllInputs BlockInputObservation.toSeparatorName (new, nullable) drives NoAction when no FREE next separator exists (train waits, retries). Stale checkBothEnds/checkOneEnd comments in operational/execution tests updated to checkAllInputs/checkInput + applyReservePath. Tests: RuleBasedDispatcherTest rewritten for 3-arg ReservePath + the null-toSeparatorName deferral; adds multi-block frozen-observation test (C1), destinationInOutName preservation (C2), same-block unreachable edge case (C3), and ShuntingLoop.failedReservationsCount counter (C4). ActuatorCommandQueueTest updated for 3-arg ReservePath. Verification: :core jvmTest 2177, :core integrationTest 721, :desktop-ui 650, :dispatcher-agent 11 + determinism 10/10 — all green; :core detekt/ktlint/checkCoreCommonMainPurity + :dispatcher-agent detekt/ktlint clean.
…ports (#737) * feat(SP0.9): sim-thread applier draining ActuatorCommandQueue via actuator ports - Add ControlStepListener fun interface to :core/sim — generic hook called on the kDisco sim thread at the start of each ShuntingLoop iteration; no dispatcher logic in :core (satisfies option-2 gate criterion). - Modify ShuntingLoop: - `var controlStepListener: ControlStepListener?` — set by wiring layer before context.run(); null by default so all existing callers are unaffected. - `fun approveQueuedTrain(trainId)` — public delegate to applyApproveTrain, used by DispatchDecisionApplier's onApproveTrain callback. - `controlStepListener?.onControlStep()` called at top of iteration() to drain and apply any pending queue decisions before the inline dispatcher runs. - Add DispatchDecisionApplier to :dispatcher-agent — implements ControlStepListener; drains ActuatorCommandQueue and routes each DispatchDecision: ApproveTrain → onApproveTrain callback, ReservePath → NetworkActuatorPort.requestRoute, NoAction → no-op. - Add DispatchDecisionApplierTest (14 tests) covering decision routing, FIFO order, empty-queue no-ops, all RouteRequestResult outcomes, and four thread-identity assertions asserting actuator calls and the approve callback always execute on the onControlStep thread (not the posting/driver thread). - Add io.mockk:mockk to :dispatcher-agent test dependencies. Closes #731. * fix(SP0.9): PR #737 review follow-up — seam test + exhaustive when Addresses the two Important issues from the PR #737 code review: * Add ShuntingLoopControlStepListenerTest (:core jvmTest, integration-tagged): - proves controlStepListener.onControlStep() is invoked exactly once per iteration, before the admission Dispatcher.decide() call (the load-bearing ordering invariant the SP0.9 design rests on) — endTime=0L gives one iteration; a custom Dispatcher records that the listener already ran. - proves approveQueuedTrain delegates to applyApproveTrain and moves a queued train into the approved set — a NoAction-only dispatcher makes the callback the only approval path; the listener consumes the captured id once (getAndSet(null)) so re-approval cannot throw in applyApproveTrain's requireNotNull and crash the kDisco process mid-iteration. Closes the conservative-sim test-coverage gap (the iteration() seam had no direct test; DispatchDecisionApplierTest only exercised onControlStep on the test thread). * Make the two `when` constructs in DispatchDecisionApplier compile-enforced exhaustive over the sealed DispatchDecision / RouteRequestResult types (expression-body `when`), matching DefaultNetworkActuatorPort.requestRoute. A future subtype (e.g. HoldTrain in SP2b, #556) is now a compile error instead of a silently-dropped decision. Also records the Minor #4 coupling observation (ApproveTrain-via-callback) as a code comment in DispatchDecisionApplier's onApproveTrain KDoc and as a comment on GitHub issue #556, recommending a TrainAdmissionPort extraction for SP2b train-lifecycle commands. Verified: :core:integrationTest 2/2, :dispatcher-agent:test + :core:jvmTest 2177/2177, detekt + ktlintCheck clean.
…ontroller (#739) * SP0.10: Add AgentLoopDriver — paced sense-decide-act loop via SimulationController * SP0.10: fix review — off-thread-safe snapshot + contract KDoc Address Important + Minor findings from PR #739 review (#732 SP0.10). Issue 2 — DefaultNetworkPerceptionPort.snapshot() was not off-thread-safe: it read plain non-volatile mutable sim fields and iterated the live approwedTrains list, so an off-thread driver call could throw a ConcurrentModificationException. Fix follows the spec's capture-on-sim-thread model (never blocks/interrupts the kDisco kernel — no sim-thread lock): - Add @volatile latestCaptured cache; captureSnapshot() does the fresh on-thread read and publishes; snapshot() returns latestCaptured or SimulationSnapshot.EMPTY — a pure cached read that never touches live state, never throws, never blocks the kernel. - Add SimulationSnapshot.EMPTY companion (default before first capture). - Migrate ShuntingLoop's two on-thread snapshot() calls (admission + path-advancement) to captureSnapshot() — behavior-preserving (still fresh reads, plus the cache publish side-effect). - AgentLoopDriver keeps calling snapshot(), now genuinely off-thread-safe. Issue 1 — contract drift: the :core KDoc on Dispatcher, SimulationController and NetworkPerceptionPort still claimed single-kDisco-thread exclusivity, contradicting the SP0.5 spec which runs the drive loop on the driver's own thread. Updated to reflect that the kDisco kernel runs on its own single thread while outside callers (the SP0.10 driver, future LLM) run in their own threads; implementations must not rely on kDisco-thread exclusivity. Minor 4 — rewrote the "mirrors DefaultSimulationContext" comment in AgentLoopDriver.runCycle() without exact numbers and without the misleading call-order claim (DefaultSimulationContext does throttle→awaitIfPaused; the driver does awaitIfPaused→throttle); kept an accurate initial-delta convention reference. Tests: added captureSnapshot() to the two SimulationSnapshotTest stubs; migrated the six fresh-read assertions in DefaultNetworkPerceptionPortTest to captureSnapshot() and added two cached-snapshot() tests (returns EMPTY before first capture; returns last capture and stays frozen through later mutations). AgentLoopDriverTest unchanged (driver uses the mocked snapshot()).
…tcher (#740) * feat(SP0.11): thin ShuntingLoop shell + Koin/DI wiring for lifted dispatcher (#733) * fix(SP0.11): suppress duplicate ReservePath race + CI green-up The async decoupling between AgentLoopDriver and the kDisco sim thread let the driver decide an identical ReservePath twice before the first application was reflected back (pathAlreadyExtendedBeyond only updates post-application). Re-applying it corrupted the train's registered path via a false-positive "legitimate circular route" match in PathReservationRegistry, permanently stalling the train. DispatchDecisionApplier now suppresses re-applying an already-reserved (trainId, from, to) triple, without touching shared navigation code. Raises RuleBasedDispatcherDeterminismTest from 0/10 to ~6/10 passing. The remaining intermittent failures are a second, pre-existing bug in PathReservationRegistry.mergePathInfo (fork-overlap vs. genuine circular-loop revisit are structurally indistinguishable by path position alone) — tracked as a follow-up, sub-issue #742 of #535. Also: add @timeout to ActuatorCommandQueueTest, AgentLoopDriverTest, DispatchDecisionApplierTest, RuleBasedDispatcherTest (none had one); cap dispatcher-agent test logging at WARN (was defaulting to a much more verbose level, producing multi-GB test-output artifacts); fix 4 ktlint violations blocking CI (import ordering in ExampleRegistry.kt and RuleBasedDispatcherDeterminismTest.kt, Koin module formatting in DispatcherAgentModule.kt, backing-property naming in ShuntingLoop.kt). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(#742): reject physically impossible switch routes + SP0.11 green-up (#743) * Add issue 742 path merge analysis * fix(#742): reject reservation of physically impossible switch routes Trace capture from failing RuleBasedDispatcherDeterminismTest runs showed the remaining ~40% failures were NOT the suspected mergePathInfo fork-overlap corruption (all 250 observed merges were clean continuations). The real defect: when a train's through-exit was momentarily blocked, findNextReservationTarget offered the sibling parallel branch's semaphore (e.g. doB1→doB2), reservePath reserved it, and configureSwitchesInPath swallowed the FATAL PathSeparatorChangeException ("switch doesn't join this segments") — returning Success for a route no configuration of switch vB can join. The train committed to an untraversable route, isPathExtendedBeyond then suppressed the corrective decision forever, and the network gridlocked (trainsExited 1 instead of 5). Fix (railway-conservative, service layer only): - configureSwitchesInPath returns Boolean: a genuinely traversed switch (both segments known) that no configuration joins now fails the candidate; null-segment cases keep the historical lenient skip (Issue #300) - configureAndRegisterSwitches orders configuration BEFORE registerSwitches - rollbackUnconfigurableCandidate undoes only the candidate's blocks and newly locked switches — the train's pre-existing path state survives, so it simply waits and reserves the through route at a future simulation time - registerPathInfo moved AFTER switch+signal configuration so no rollback can leave a poisoned PathInfo Circular-route merging is untouched: mergePathInfo/addElementWithCycleDetection unchanged; Issue316RegressionTest and PathReservationRegistryMergingTest pass unchanged. New Issue742RegressionTest (@timeout): impossible diversion rejected without leaking blocks/locks/PathInfo (failed pre-fix), wait-then-extend partial-path semantics, legitimate parallel alternate zB→doA2 still succeeds. Determinism evidence on the dev machine: pre-fix 55/100 failed repetitions (0/10 suites green); post-fix 8/210 failed repetitions, all matching the pre-existing headless driver-pacing race (trainsExited=0 occurred 22×/100 pre-fix too) — that residue is AgentLoopDriver pacing, tracked separately. Closes #742 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(SP0.11): green-up clean build integrationTest — KMP break, dispatcher wiring, pacing tests Four SP0.11 (thin ShuntingLoop shell, #733) regressions fixed; none were covered by CI because this branch's workflow run was never approved (goal-10's last green CI is SP0.10). 1. KMP metadata compile break: ShuntingLoop.startAction used JVM-only Thread/runBlocking in commonMain. New expect/actual platformStartDaemonThread (util/PlatformThread.kt): JVM = daemon Thread + runBlocking; native (linuxX64) = CoroutineScope(newSingleThreadContext). Local gemma4 review suggested kotlin.concurrent.thread(isDaemon=true) — rejected: that API is JVM-only, does not exist on Kotlin/Native. 2. Zero-train runs everywhere the async :dispatcher-agent stack isn't wired: SP0.11 removed inline admission/reservation policy but only desktop-ui's ExampleRegistry got the replacement wiring. New sim/SynchronousDispatcherWiring.wireSynchronousDispatcher(env, loop) — production RuleBasedDispatcher + core ports, deciding and applying synchronously on the sim thread each iteration (pre-SP0.11 semantics; deterministic by construction, immune to the PR #740/#742 async races). Used by :fast-sim production (Main, NativeExampleRegistry — fixes 7 native integration tests + JVM/native parity) and by the bare-ShuntingLoop test suites (JvmParity, ShuntingLoopRegression, TrainReporter, Metrics, TrainCoverage, KoinGoldenOutput, SimulationSpeedGolden). Golden baselines pass unchanged — the synchronous wiring reproduces pre-SP0.11 outputs. 3. AgentLoopDriverTest pacing tests updated to the SP0.11 stagnant-snapshot contract runCycle gained (skip decide/throttle on a re-read of the same sim tick, pause still honoured): multi-cycle test now advances simTime per cycle; zero-delta test replaced by stagnantSimTimeSkipsCycle pinning the skip behaviour (no re-decide — that re-decide was the #740 race source). 4. RuleBasedDispatcherDeterminismTest flaked (~1 in 3 suite runs) because the free-running driver thread races the unthrottled headless sim — admission timing drifted with OS scheduling. The Goal 10 A3 gate asserts determinism of the DISPATCHER, so executeRun now runs a lock-step handshake: the sim thread grants the driver exactly one cycle per tick and drains its decisions in the same tick, all on production components. 5 consecutive suite runs green (50/50 repetitions). Verified: ./gradlew clean build integrationTest --no-build-cache (122 tasks, 120 executed) BUILD SUCCESSFUL — includes :core jvm+native tests, :core purity gate, :desktop-ui, :dispatcher-agent, :fast-sim linuxX64 tests, detekt, ktlint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(docker): copy dispatcher-agent build script and sources into image settings.gradle.kts includes :dispatcher-agent, but the Dockerfile's layered COPY steps never picked it up alongside core/core-test/desktop-ui. Gradle saw an included project with no build script or sources, giving it zero variants and breaking desktop-ui's runtimeClasspath resolution ("Could not resolve project :dispatcher-agent ... No variants exist."). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Bedrich Hovorka <bedrich.hovorka@gmail.com> * Delete docs/analysis/ANALYSIS-742.md * Add PR #743 new-code coverage tests for Issue #742 fix (4 new tests covering uncovered branches) * fix(SP0.11): address PR #740 review findings #2-#7 Code review of PR #740 (vs origin/goal-10) found 7 Important issues. #8 (pre-existing ~4% AgentLoopDriver pacing determinism residual) is tracked separately as SP0.11c (#746), not fixed here. #2 ShuntingLoop: consolidate three @volatile observation fields into one atomic TickObservation snapshot, so the off-kernel driver thread always reads a consistent same-tick set instead of a cross-tick mix that could trip a stale pathAlreadyExtendedBeyond and a duplicate re-decide. #3 ExampleRegistry.wireDispatcherAgent: resolve Dispatcher + ActuatorCommandQueue from context.scope via Koin (Goal 10 seam) instead of constructing RuleBasedDispatcher/ActuatorCommandQueue directly; add dispatcherAgentModule to testModuleLightweight + integrationTestModule so the resolution works in every test harness. #4/#5 KDoc: replace stale "two-phase tick" / "pre-SP0.11 timing" docs with the SP0.11 single-observation-per-tick model (DispatchObservation, Dispatcher, RuleBasedDispatcher, SynchronousDispatcherWiring, ShuntingLoop). #6 DefaultPathReservationService: signal-config failure now uses the scoped rollbackUnconfigurableCandidate instead of a full registry.unregister that would nuke a mid-journey train's pre-existing path; add a per-switch PathReservationRegistry.unregisterSwitch primitive; thread priorSwitches so earlier-hop switches survive; remove the now-dead rollbackCompleteReservation. #7 DefaultPathReservationService: unconfigurable-switch failure now `continue`s to the next candidate like the other failure modes (was `return AllPathsBlocked`). Extracted the Step 2g signal-config `when` into a configureStartSignal helper to keep reservePath under the detekt cyclomatic-complexity threshold. Tests: new unregisterSwitchReleasesOneSwitchAndKeepsTheRest unit test in Issue742RegressionTest; refreshed stale SignalConfigurationRollbackTest comments pointing at the removed rollbackCompleteReservation. Gates green: :core:checkCoreCommonMainPurity, :core:jvmTest (2187/2187), :dispatcher-agent:test, :desktop-ui:test, :desktop-ui:integrationTest (244 passed / 3 skipped / 0 failed), detekt, ktlintCheck. Co-Authored-By: Claude <noreply@anthropic.com> * docs(SP0.11): Update comment on Dispatcher binding to mention future Agentic/LLM variant
…he 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>
|
…/CD and SonarQube configuration (#748) * SP1.1: Add dispatcher-agent to test, integration test, and SonarQube configuration * SP1.1: Configure JaCoCo XML report generation for dispatcher-agent
…lity (~9% fast-sim win) (#754) * fix(#749): gate findNextReservationTarget on reservation-eligibility ShuntingLoop.toBlockInputObservation resolved BlockInputObservation.toSeparatorName by calling PathReservationService.findNextReservationTarget for every block input on every tick, including inputs that provably cannot take a forward reservation. The call is a BFS (findNextSemaphoresVia) plus a full topological-path enumeration per candidate (isPathAvailable -> findAllTopologicalPaths). Measured on vyhybna: 906 calls per `example shuntingLoop 300`, of which 892 (98.5%) are discarded unread by RuleBasedDispatcher.checkInput, which already returns null for FREE, not-approaching and already-extended inputs. Cost: 30.3 ms of 342 ms wall (~9%). Gate the search on eligibility. The three facts it needs (isApproachingThisInput, pathSetUpTowardThisInput, pathAlreadyExtendedBeyond) are already computed in the same function, so the gate is free. This narrows the sensor-port contract: toSeparatorName is now populated only where a forward reservation is possible, and is null elsewhere without the search having run. RuleBasedDispatcher behaviour is unchanged by construction. Documented on BlockInputObservation so a future LLM dispatcher does not read null as "track ahead is occupied". Native linuxX64 release, example shuntingLoop 300, median of 7: before 341 ms after 318 ms Golden output byte-for-byte identical (90/90 event lines, 5 trains, 211.1 s sim time). ShuntingLoopReservationTargetLazinessTest locks the contract and fails 2/2 without the gate (a FREE input resolves k1->doA1=A). Gates: core:jvmTest 2194, core:integrationTest 725, core:heavyTest 1001, dispatcher-agent:test 51, desktop-ui:test 650, desktop-ui:integrationTest 247 — all green. purity/detekt/ktlint clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Extract reservation-target helper --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
… JMH `:core:integrationTest` failed on run 29165254178 (725 tests, 1 failed). The failure was DefaultSimulationContextControllerTest.controlledLoopOverheadIsNegligible, a wall-clock microbenchmark living inside the CI correctness gate: it timed one 300s ShuntingLoop run per arm and asserted the ratio was < 5%. It measured 5.33%. It could not have held up. One sample per arm, no JIT warmup (the baseline arm ran first and absorbed it, biasing the ratio negative, so it passed for a reason unrelated to the property it claimed to test), and a 5% threshold on a shared runner whose CPU-steal variance far exceeds 5%. Note TwoTrainConcurrencyTest passed 150/150 in that run and is not implicated; the simulation is a single-threaded kDisco discrete-event loop with a seeded RNG, so there is no race here to serialize. - Delete the wall-clock assertion. The class keeps its four deterministic tests (throttle invoked per event, non-negative deltas, StopSimulation propagates, awaitIfPaused called), which is what a correctness gate should assert. - Add ControlledLoopOverheadBenchmark to the existing :desktop-ui JMH harness, with two independently reported arms instead of one fragile ratio. Only ctx.run(controller) is timed; context construction sits in @setup(Level.Invocation). Mode is SingleShotTime: the payload is a single-use, millisecond-scale run, and under AverageTime the resulting context churn collides Koin scope ids, since DefaultSimulationContext derives its scope id from a (non-unique) identity hash code. Measured: baseline 0.699 +/- 0.103 ms/op, with-controller 0.685 +/- 0.062 ms/op. Indistinguishable -- the hook overhead is below the noise floor, so the original claim was true; only the instrument was broken. - Fix a latent order-dependent bug in TwoTrainConcurrencyTest found en route. It matched train names by substring, and Train.countValue is a process-global counter that is never reset, so once names reach two digits "Train #1" matches "Train #12". Now compares extracted whole tokens and indexes by identity rather than List.indexOf. Verified: :core:integrationTest 724/724 (was 725/1 failed), :core:jvmTest 2194/2194, :desktop-ui:test + integrationTest green, detekt + ktlintCheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TODO