From 28768516d5fbacf74e3de74fea33a9b7128654db Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:14:34 +0200 Subject: [PATCH 1/5] fix(proxy): clear aircraft on the RESET topic, not on an ACDATA action BlueSky signals a simulation reset on the dedicated RESET topic, but the clear lived in on_acdata_received, keyed off a context action that the client synthesised from the shared-state wrapper. That mapping made the ACDATA path clear traffic on frames it should only have forwarded, while a real RESET left proxy.traffic_data populated. - bluesky_client: stop rewriting the wire action into Reset/ActChange; record it verbatim and hand handlers the unwrapped data dict. - events.on_reset_received: clear the cached traffic and emit an empty acdata alongside the existing poly/polyline clears, still scoped to the active node. The cache is cleared even with no browser attached, since on_acdata_received only refreshes it while clients are connected and a stale cache would be served verbatim in the next initial_data snapshot. - node_manager: drop the previous node's cached traffic when the active node changes, so it can't be re-served before the new node's first frame. Co-Authored-By: Claude Opus 5 (1M context) --- WebATM/bluesky_client.py | 21 +++--------- WebATM/proxy/handlers/events.py | 31 ++++++++++++------ WebATM/proxy/handlers/simulation.py | 37 ++++++--------------- WebATM/proxy/managers/node_manager.py | 11 +++++-- tests/test_bluesky_client.py | 14 ++++++++ tests/test_handlers.py | 46 ++++++++++++++++++++++----- tests/test_node_manager.py | 45 ++++++++++++++++++++++++++ 7 files changed, 142 insertions(+), 63 deletions(-) diff --git a/WebATM/bluesky_client.py b/WebATM/bluesky_client.py index bc8431b..9855141 100644 --- a/WebATM/bluesky_client.py +++ b/WebATM/bluesky_client.py @@ -699,26 +699,15 @@ def _process_data_message(self, msg): # Pass sender_id as additional parameter to our custom handler self.subscriber.emit(topic, *data, sender_id=sender_id) elif topic in ("ACDATA", "ROUTEDATA"): - # ACDATA and ROUTEDATA - handle BlueSky shared state format + # ACDATA and ROUTEDATA use the BlueSky shared-state format + # [action_type, data_dict]; record the wire action ('R', + # 'U', ...) and sender on the context, then pass handlers + # the unwrapped data dict. if isinstance(data, (list, tuple)) and len(data) == 2: - # BlueSky shared state format: [action_type, data_dict] action_type, actual_data = data - - # Set context action for BlueSky compatibility self.context.action = action_type self.context.sender_id = sender_id - - # Handle different action types like BlueSky does - if action_type in ("RESET", "ACTCHANGE"): - self.context.action = ( - self.context.Reset - if action_type == "RESET" - else self.context.ActChange - ) - - self.subscriber.emit( - topic, actual_data - ) # Pass the actual data dict, not the action wrapper + self.subscriber.emit(topic, actual_data) else: self.subscriber.emit(topic, data) # Pass as single argument elif topic == "ECHO": diff --git a/WebATM/proxy/handlers/events.py b/WebATM/proxy/handlers/events.py index ac28201..5c50a72 100644 --- a/WebATM/proxy/handlers/events.py +++ b/WebATM/proxy/handlers/events.py @@ -3,7 +3,7 @@ import time from ...logger import get_logger -from ...utils import id2str +from ...utils import empty_traffic_data, id2str from ._base import active_proxy logger = get_logger() @@ -13,12 +13,13 @@ def on_reset_received(data=None, *args, sender_id=None, **kwargs): """Handle RESET events from the BlueSky server. Clears the stored polygon/polyline shapes for the node that sent the - reset. Browsers display the active node only, so the map-clearing ``poly`` - and ``polyline`` payloads and the ``reset`` event are emitted solely when - the resetting node is the active one — a background node's reset must not - wipe the active node's display. When the sender or active node can't be - resolved, the reset is accepted so a single-node display still works - (same fallback as the SIMINFO/ACDATA active-node filter). + reset. Browsers display the active node only, so when the resetting node + is the active one this also clears the cached aircraft and emits the + map-clearing ``acdata``/``poly``/``polyline`` payloads plus the ``reset`` + event — while a background node's reset must not wipe the active node's + display. When the sender or active node can't be resolved, the reset is + accepted so a single-node display still works (same fallback as the + SIMINFO/ACDATA active-node filter). Args: data (Any): Optional RESET payload (unused). @@ -26,8 +27,7 @@ def on_reset_received(data=None, *args, sender_id=None, **kwargs): sender_id (bytes | str | None): Node that reset, from the message header; bytes are converted to a hex string. The shared network context is deliberately not consulted — it holds the sender of the - last shared-state message (usually the active node), not of this - RESET. + last shared-state message, not of this RESET. **kwargs (Any): Additional keyword payload items (unused). """ proxy = active_proxy() @@ -49,7 +49,18 @@ def on_reset_received(data=None, *args, sender_id=None, **kwargs): is_active_node = ( active_node_id is None or sender_id is None or sender_id == active_node_id ) - if is_active_node and proxy.socketio and proxy.connected_clients > 0: + if not is_active_node: + return + + # The active node's cached aircraft are gone too. Clear even with no + # browser connected: on_acdata_received only refreshes this cache + # while clients are attached, so a stale cache would otherwise be + # served verbatim in the next initial_data snapshot (ghost aircraft). + cleared = empty_traffic_data() + proxy.traffic_data = cleared + + if proxy.socketio and proxy.connected_clients > 0: + proxy.socketio.emit("acdata", cleared) proxy.socketio.emit("poly", {"polys": {}}) proxy.socketio.emit("polyline", {"polys": {}}) proxy.socketio.emit( diff --git a/WebATM/proxy/handlers/simulation.py b/WebATM/proxy/handlers/simulation.py index d4ddc92..a62eaa7 100644 --- a/WebATM/proxy/handlers/simulation.py +++ b/WebATM/proxy/handlers/simulation.py @@ -10,7 +10,7 @@ import time from ...logger import get_logger -from ...utils import empty_traffic_data, id2str, make_json_serializable, tim2txt +from ...utils import id2str, make_json_serializable, tim2txt from ..perf import data_path_perf from ._base import active_proxy, get_bluesky_proxy @@ -107,16 +107,16 @@ def on_siminfo_received( def on_acdata_received(data): """Process a BlueSky ACDATA traffic frame and emit ``acdata`` to web clients. - On a simulation reset or active-node change (detected via the BlueSky - network context), clears the cached traffic data and immediately emits an - empty ``acdata`` payload so browsers drop stale aircraft. - Hot path: the network timer delivers ACDATA at up to 50 Hz, but it is only emitted to browsers at ``acdata_interval`` (10 Hz) and only for the active node. ``make_json_serializable`` is the dominant per-frame cost, so it is deferred until after the active-node filter and the emit throttle decide the frame is actually sent. Set WEBATM_PERF=1 (WebATM.proxy.perf) to measure it. + Aircraft clearing on a simulation reset is NOT handled here: BlueSky + signals a reset on the dedicated RESET topic (events.on_reset_received), + never via an ACDATA action. + Args: data (dict): Aircraft state arrays keyed by field (``id``, ``lat``, ``lon``, ``alt``, ``tas``, ``trk``, ``vs``, conflict counters, ...) @@ -133,30 +133,13 @@ def on_acdata_received(data): logger.debug("on_acdata_received ignored - reconnection not allowed") return - # Check context action like BlueSky web client does, and resolve which - # node sent this frame (set on the shared context just before this - # synchronous dispatch) for the active-node filter below. + # Resolve which node sent this frame (set on the shared context just + # before this synchronous dispatch) for the active-node filter below. sender_id_str = None if proxy.bluesky_client and hasattr(proxy.bluesky_client, "context"): - ctx = proxy.bluesky_client.context - if ctx.action == ctx.Reset or ctx.action == ctx.ActChange: - # Simulation reset or active-node change: clear all aircraft. - logger.info("ACDATA reset/actchange detected - clearing aircraft data") - cleared = empty_traffic_data() - proxy.traffic_data = cleared - - # Emit cleared data immediately - if proxy.socketio and proxy.connected_clients > 0: - try: - proxy.socketio.emit("acdata", cleared) - logger.debug( - f"Emitted cleared ACDATA to {proxy.connected_clients} web clients" - ) - except Exception as e: - logger.error(f"Error emitting cleared ACDATA: {e}") - return - - sender_id_str = id2str(getattr(ctx, "sender_id", None)) + sender_id_str = id2str( + getattr(proxy.bluesky_client.context, "sender_id", None) + ) # Any ACDATA from any node proves the link to BlueSky is alive: update # liveness before filtering so a background node's traffic still counts. diff --git a/WebATM/proxy/managers/node_manager.py b/WebATM/proxy/managers/node_manager.py index 4a9e20d..390ee11 100644 --- a/WebATM/proxy/managers/node_manager.py +++ b/WebATM/proxy/managers/node_manager.py @@ -6,7 +6,7 @@ from ...bluesky_client import safe_decode, seqid2idx, seqidx2id from ...logger import get_logger -from ...utils import id2str +from ...utils import empty_traffic_data, id2str logger = get_logger() @@ -49,7 +49,14 @@ def _get_safe_active_node(self): def _on_actnode_changed(self, node_id): """Callback when active node changes.""" if self.proxy.running: - # Emit immediately to update web interface + # The cached traffic belongs to the previous node; drop it (and + # clear browsers) so it can't be re-served while the new node's + # ACDATA stream spins up. Same idea as the shape clear below. + cleared = empty_traffic_data() + self.proxy.traffic_data = cleared + if self.proxy.socketio and self.proxy.connected_clients > 0: + self.proxy.socketio.emit("acdata", cleared) + self._emit_node_info() # Emit POLY data for the newly active node diff --git a/tests/test_bluesky_client.py b/tests/test_bluesky_client.py index 744e85b..b194a15 100644 --- a/tests/test_bluesky_client.py +++ b/tests/test_bluesky_client.py @@ -347,6 +347,20 @@ def test_reset_threads_sender_from_header(self): assert client.context.sender_id == b"NODE\x82" assert client.context.action == client.context.Reset + def test_acdata_shared_state_unwraps_and_keeps_wire_action(self): + # ACDATA arrives as [action, data]; the handler gets the unwrapped + # dict and the context records the raw wire action and header sender. + client = BlueSkyClient() + received = [] + client.subscriber.subscribe("ACDATA", lambda data: received.append(data)) + + traffic = {"id": ["AC1"], "lat": [52.0]} + client._process_data_message(self._frame("ACDATA", [b"R", traffic])) + + assert received == [traffic] + assert client.context.action == b"R" + assert client.context.sender_id == b"NODE\x81" + def test_generic_topic_still_dispatches(self): client = BlueSkyClient() received = [] diff --git a/tests/test_handlers.py b/tests/test_handlers.py index da1b5b4..8b2d2b8 100644 --- a/tests/test_handlers.py +++ b/tests/test_handlers.py @@ -175,16 +175,13 @@ def test_ignored_when_reconnection_disallowed(self, proxy, fake_socketio): on_acdata_received({"id": ["AC1"]}) assert proxy.traffic_data == {} - def test_reset_clears_aircraft(self, proxy, fake_client, fake_socketio): + def test_wire_action_does_not_clear_aircraft(self, proxy, fake_client): + # Reset clearing is the RESET topic's job (on_reset_received); a + # shared-state action on the ACDATA context must not wipe traffic. proxy.bluesky_client = fake_client - fake_client.context.action = fake_client.context.Reset + fake_client.context.action = "R" on_acdata_received({"id": ["AC1"]}) - emitted = fake_socketio.last("acdata") - assert emitted["id"] == [] - # Canonical empty payload shape (matches the disconnect clear path). - from WebATM.utils import empty_traffic_data - - assert emitted == empty_traffic_data() + assert proxy.traffic_data["id"] == ["AC1"] class TestAcdataActiveNodeFiltering: @@ -414,6 +411,30 @@ def test_clears_shapes_and_emits(self, proxy, fake_socketio): assert sender_hex not in proxy.poly_data_by_node assert fake_socketio.count("reset") == 1 + def test_clears_cached_aircraft_and_emits_empty_acdata(self, proxy, fake_socketio): + from WebATM.utils import empty_traffic_data + + proxy.traffic_data = {"id": ["GHOST1", "GHOST2"], "lat": [1.0, 2.0]} + on_reset_received(sender_id=b"NODE1") + # Canonical empty payload shape (matches the disconnect clear path). + assert proxy.traffic_data == empty_traffic_data() + assert fake_socketio.last("acdata") == empty_traffic_data() + + def test_clears_cached_aircraft_with_no_clients_connected( + self, proxy, fake_socketio + ): + # The initial_data snapshot serves proxy.traffic_data verbatim, and + # on_acdata_received only refreshes it while clients are attached — so + # a reset with nobody connected must still clear the cache, or the + # next page load renders the pre-reset aircraft (ghosts). + from WebATM.utils import empty_traffic_data + + proxy.connected_clients = 0 + proxy.traffic_data = {"id": ["GHOST1"], "lat": [1.0]} + on_reset_received(sender_id=b"NODE1") + assert proxy.traffic_data == empty_traffic_data() + assert fake_socketio.count("acdata") == 0 # nobody to emit to + def test_ignored_when_reconnection_disallowed(self, proxy, fake_socketio): proxy.allow_reconnection = False on_reset_received() @@ -449,6 +470,7 @@ def test_background_node_reset_leaves_active_display_alone( other_hex: {"polys": {"stale": {}}}, } proxy.polyline_data_by_node = {active_hex: {"polys": {"keep": {}}}} + proxy.traffic_data = {"id": ["ACTIVE1"], "lat": [1.0]} on_reset_received(sender_id=other) @@ -456,22 +478,30 @@ def test_background_node_reset_leaves_active_display_alone( assert other_hex not in proxy.poly_data_by_node assert proxy.poly_data_by_node[active_hex] == {"polys": {"keep": {}}} assert proxy.polyline_data_by_node[active_hex] == {"polys": {"keep": {}}} + # The active node's cached aircraft survive a background reset. + assert proxy.traffic_data == {"id": ["ACTIVE1"], "lat": [1.0]} # Nothing display-clearing reaches the browser. assert fake_socketio.count("reset") == 0 + assert fake_socketio.count("acdata") == 0 assert fake_socketio.count("poly") == 0 assert fake_socketio.count("polyline") == 0 def test_active_node_reset_clears_and_emits( self, proxy, fake_client, fake_socketio ): + from WebATM.utils import empty_traffic_data + active = b"\xaa\xaa\xaa\xaa\x81" active_hex = self._activate(proxy, fake_client, active) proxy.poly_data_by_node = {active_hex: {"polys": {"stale": {}}}} + proxy.traffic_data = {"id": ["OLD1"], "lat": [1.0]} on_reset_received(sender_id=active) assert active_hex not in proxy.poly_data_by_node + assert proxy.traffic_data == empty_traffic_data() assert fake_socketio.count("reset") == 1 + assert fake_socketio.last("acdata") == empty_traffic_data() assert fake_socketio.last("poly") == {"polys": {}} assert fake_socketio.last("polyline") == {"polys": {}} diff --git a/tests/test_node_manager.py b/tests/test_node_manager.py index 6ace317..7cdfaa5 100644 --- a/tests/test_node_manager.py +++ b/tests/test_node_manager.py @@ -305,6 +305,51 @@ def test_no_emit_without_clients(self, proxy, fake_client, fake_socketio): assert fake_socketio.count("polyline") == 0 +class TestActnodeChangedTrafficClear: + """Switching the active node drops the previous node's cached aircraft: + the cache would otherwise be re-served (initial_data snapshot, backup + emit) until the new node's ACDATA stream produces its first frame.""" + + def test_switch_clears_cached_traffic_and_emits_empty_acdata( + self, proxy, fake_client, fake_socketio + ): + from WebATM.utils import empty_traffic_data + + proxy.bluesky_client = fake_client + proxy.running = True + proxy.traffic_data = {"id": ["OLDNODE1"], "lat": [1.0]} + + proxy.node_mgr._on_actnode_changed(b"\x01\x02\x03\x04\x81") + + assert proxy.traffic_data == empty_traffic_data() + assert fake_socketio.last("acdata") == empty_traffic_data() + + def test_switch_clears_cache_even_without_clients( + self, proxy, fake_client, fake_socketio + ): + from WebATM.utils import empty_traffic_data + + proxy.bluesky_client = fake_client + proxy.running = True + proxy.connected_clients = 0 + proxy.traffic_data = {"id": ["OLDNODE1"], "lat": [1.0]} + + proxy.node_mgr._on_actnode_changed(b"\x01\x02\x03\x04\x81") + + assert proxy.traffic_data == empty_traffic_data() + assert fake_socketio.count("acdata") == 0 + + def test_not_running_is_a_noop(self, proxy, fake_client, fake_socketio): + proxy.bluesky_client = fake_client + proxy.running = False + proxy.traffic_data = {"id": ["KEEP"], "lat": [1.0]} + + proxy.node_mgr._on_actnode_changed(b"\x01\x02\x03\x04\x81") + + assert proxy.traffic_data == {"id": ["KEEP"], "lat": [1.0]} + assert fake_socketio.count("acdata") == 0 + + class TestDelegationToNetworkClient: def test_actnode_raises_without_client(self, proxy): proxy.bluesky_client = None From e8be197ae798145ee566681e68622d61e24a7a4b Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:14:46 +0200 Subject: [PATCH 2/5] fix(console): keep the unsubmitted draft across history navigation ArrowUp discarded whatever was typed on the fresh input line, and ArrowDown back past the newest entry restored an empty string instead of it, so a half-written command was lost by glancing at history. Shells stash the draft; CommandHistory now does too. Also skip consecutive duplicate entries on add(), reset navigation on submit, and fold the two near-identical arrow-key branches in Console into one navigateHistory() call that passes the current input through. Console.historyNav.test.ts drives the real keydown handler end to end so the round trip is pinned at the DOM level, not just on the model. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/ui/CommandHistory.test.ts | 29 +++++++ frontend/src/ui/CommandHistory.ts | 43 ++++++----- frontend/src/ui/Console.historyNav.test.ts | 89 ++++++++++++++++++++++ frontend/src/ui/Console.ts | 51 +++---------- 4 files changed, 155 insertions(+), 57 deletions(-) create mode 100644 frontend/src/ui/Console.historyNav.test.ts diff --git a/frontend/src/ui/CommandHistory.test.ts b/frontend/src/ui/CommandHistory.test.ts index 236d9b5..69bff58 100644 --- a/frontend/src/ui/CommandHistory.test.ts +++ b/frontend/src/ui/CommandHistory.test.ts @@ -26,6 +26,11 @@ describe('CommandHistory', () => { expect(history.entries).toEqual(['b', 'c', 'd']); }); + it('skips consecutive duplicates but keeps non-consecutive ones', () => { + ['OP', 'OP', 'HOLD', 'OP'].forEach(cmd => history.add(cmd)); + expect(history.entries).toEqual(['OP', 'HOLD', 'OP']); + }); + it('persists across instances via storage', () => { history.add('CRE KL123'); @@ -72,5 +77,29 @@ describe('CommandHistory', () => { const empty = new CommandHistory('empty-history'); expect(empty.previous()).toBeNull(); }); + + it('restores the draft passed to previous() when next() steps past the newest', () => { + expect(history.previous('CRE KL204 B7')).toBe('third'); + expect(history.previous()).toBe('second'); + expect(history.next()).toBe('third'); + expect(history.next()).toBe('CRE KL204 B7'); // draft back on the fresh line + expect(history.next()).toBeNull(); + }); + + it('only stashes the draft when leaving the fresh line, not mid-navigation', () => { + history.previous('my draft'); // third + history.previous('not a draft'); // second - already navigating + history.next(); // third + expect(history.next()).toBe('my draft'); + }); + + it('add() during navigation resets it so previous() starts at the newest entry', () => { + history.previous('draft'); // third + history.previous(); // second + history.add('fourth'); // e.g. a map-drawn aircraft via displaySentCommand + expect(history.previous()).toBe('fourth'); + history.resetNavigation(); + expect(history.next()).toBeNull(); // draft was discarded too + }); }); }); diff --git a/frontend/src/ui/CommandHistory.ts b/frontend/src/ui/CommandHistory.ts index 9174cb9..0e8ee11 100644 --- a/frontend/src/ui/CommandHistory.ts +++ b/frontend/src/ui/CommandHistory.ts @@ -6,14 +6,15 @@ import { logger } from '../utils/Logger'; * up/down-arrow navigation, extracted from Console so the navigation * semantics are testable without a DOM. * - * Navigation model: a null index means "fresh input line". previous() - * walks toward older commands and sticks at the oldest (no cycling); - * next() walks toward newer commands and finally returns to the fresh - * line (empty string). + * Navigation model (matches a shell): a null index means "fresh input + * line". previous() walks toward older commands and sticks at the oldest + * (no cycling); next() walks toward newer commands and finally restores + * the draft that was on the fresh line when navigation started. */ export class CommandHistory { private history: string[] = []; private index: number | null = null; // null means fresh input line + private draft: string | null = null; // fresh-line text stashed while navigating constructor( private readonly storageKey: string = 'console-command-history', @@ -29,40 +30,47 @@ export class CommandHistory { } } - /** Append a command (newest last), cap the size, and persist. */ + /** + * Append a command (newest last), cap the size, and persist. + * Skips blank input and consecutive duplicates, and ends any + * in-progress navigation so the next ArrowUp starts at the newest + * entry again. + */ add(command: string): void { + this.resetNavigation(); if (!command.trim()) return; + if (this.history[this.history.length - 1] === command) return; this.history.push(command); if (this.history.length > this.maxEntries) { - this.history.shift(); // Remove oldest command + this.history.shift(); } storage.set(this.storageKey, this.history); } /** * Step to the previous (older) command. Returns the command to show, - * or null when there is no history to navigate. + * or null when there is no history to navigate. `currentInput` is + * stashed as the draft when this step leaves the fresh input line. */ - previous(): string | null { + previous(currentInput: string = ''): string | null { if (this.history.length === 0) return null; - // If we're at fresh input state, go to newest command if (this.index === null) { + this.draft = currentInput; this.index = this.history.length - 1; } else if (this.index > 0) { - // Move to older command this.index--; } - // If already at oldest command, stay there (no cycling) + // else: already at the oldest command - stay there (no cycling) return this.history[this.index]; } /** - * Step to the next (newer) command. Returns the command to show, - * '' when stepping past the newest back to the fresh input line, or - * null when not currently navigating. + * Step to the next (newer) command. Returns the command to show, the + * stashed draft when stepping past the newest back to the fresh input + * line, or null when not currently navigating. */ next(): string | null { if (this.history.length === 0 || this.index === null) { @@ -74,14 +82,15 @@ export class CommandHistory { return this.history[this.index]; } - // At newest command, go back to fresh input - this.index = null; - return ''; + const draft = this.draft ?? ''; + this.resetNavigation(); + return draft; } /** Return to the fresh-input state (e.g. after submitting a command). */ resetNavigation(): void { this.index = null; + this.draft = null; } get entries(): readonly string[] { diff --git a/frontend/src/ui/Console.historyNav.test.ts b/frontend/src/ui/Console.historyNav.test.ts new file mode 100644 index 0000000..2a1b753 --- /dev/null +++ b/frontend/src/ui/Console.historyNav.test.ts @@ -0,0 +1,89 @@ +// @vitest-environment happy-dom +/** + * Console arrow-key history navigation, wired end-to-end through the real + * keydown handler. Pins the fix for the draft-loss bug: an unsubmitted + * command must survive an ArrowUp (recall history) → ArrowDown (come back) + * round trip instead of being wiped to an empty input. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { Console } from './Console'; + +function setupDom(): void { + document.body.innerHTML = ` +
+ +
+
+ BS> + + +
+
+ `; +} + +function input(): HTMLInputElement { + return document.getElementById('console-input') as HTMLInputElement; +} + +function press(key: string): void { + input().dispatchEvent(new KeyboardEvent('keydown', { key, bubbles: true, cancelable: true })); +} + +function submit(command: string): void { + input().value = command; + (document.getElementById('send-command') as HTMLButtonElement).click(); +} + +describe('Console arrow-key history navigation', () => { + const app = { sendCommand: vi.fn() }; + let konsole: Console; + + beforeEach(() => { + localStorage.clear(); + setupDom(); + app.sendCommand.mockReset(); + window.app = app as unknown as Window['app']; + konsole = new Console(); + submit('MCRE 2'); + submit('OP'); + }); + + afterEach(() => { + delete window.app; + document.body.innerHTML = ''; + }); + + it('ArrowUp recalls the newest command, ArrowUp again the older one', () => { + press('ArrowUp'); + expect(input().value).toBe('OP'); + press('ArrowUp'); + expect(input().value).toBe('MCRE 2'); + }); + + it('preserves an unsubmitted draft across an ArrowUp/ArrowDown round trip', () => { + input().value = 'CRE KL204 B744 52.3 4.9'; + press('ArrowUp'); + expect(input().value).toBe('OP'); + press('ArrowDown'); + expect(input().value).toBe('CRE KL204 B744 52.3 4.9'); + }); + + it('a command recorded mid-navigation restarts ArrowUp at the newest entry', () => { + press('ArrowUp'); // OP + press('ArrowUp'); // MCRE 2 + konsole.displaySentCommand('HOLD'); // e.g. a map-drawn aircraft + press('ArrowUp'); + expect(input().value).toBe('HOLD'); + }); + + it('submitting resets navigation and the draft', () => { + input().value = 'a draft'; + press('ArrowUp'); // OP, draft stashed + submit('HOLD'); + press('ArrowUp'); + expect(input().value).toBe('HOLD'); + press('ArrowDown'); + expect(input().value).toBe(''); // draft was consumed by the submit + }); +}); diff --git a/frontend/src/ui/Console.ts b/frontend/src/ui/Console.ts index c4bacac..f065cfe 100644 --- a/frontend/src/ui/Console.ts +++ b/frontend/src/ui/Console.ts @@ -88,9 +88,6 @@ export class Console { inputContainer.parentElement.insertBefore(this.argHint, inputContainer); } - /** - * Set state manager reference to access command dictionary - */ public setStateManager(stateManager: StateManager): void { this.stateManager = stateManager; } @@ -105,9 +102,6 @@ export class Console { return getEffectiveDict(this.stateManager?.getCommandDict() ?? null); } - /** - * Set command handler reference for processing local commands - */ public setCommandHandler(commandHandler: CommandHandler): void { this.commandHandler = commandHandler; } @@ -120,9 +114,6 @@ export class Console { this.mapPicker = new ConsoleMapPicker(mapDisplay, this, navaidSnapper); } - /** - * Create suggestion overlay element - */ private createSuggestionOverlay(): void { const inputContainer = document.querySelector('.console-input-container'); if (!inputContainer) { @@ -153,15 +144,9 @@ export class Console { switch (e.key) { case 'ArrowUp': - e.preventDefault(); - this.showPreviousCommand(); - this.updateSuggestion(); - this.updateArgHint(); - this.updateMapPicker(); - break; case 'ArrowDown': e.preventDefault(); - this.showNextCommand(); + this.navigateHistory(e.key === 'ArrowUp' ? 'previous' : 'next'); this.updateSuggestion(); this.updateArgHint(); this.updateMapPicker(); @@ -335,21 +320,18 @@ export class Console { } } - private showPreviousCommand(): void { - const input = document.getElementById('console-input') as HTMLInputElement; - if (!input) return; - - const command = this.history.previous(); - if (command !== null) { - input.value = command; - } - } - - private showNextCommand(): void { + /** + * Arrow-key history walk. previous() gets the current input so an + * unsubmitted draft survives an ArrowUp/ArrowDown round trip. + */ + private navigateHistory(direction: 'previous' | 'next'): void { const input = document.getElementById('console-input') as HTMLInputElement; if (!input) return; - const command = this.history.next(); + const command = + direction === 'previous' + ? this.history.previous(input.value) + : this.history.next(); if (command !== null) { input.value = command; } @@ -452,7 +434,7 @@ export class Console { const prompt = document.querySelector('.console-prompt') as HTMLElement; if (input && prompt) { - // Calculate position based on input text width + // Measure the typed text so the overlay starts just past it. const canvas = document.createElement('canvas'); const context = canvas.getContext('2d'); if (context) { @@ -460,24 +442,16 @@ export class Console { context.font = computedStyle.font; const textWidth = context.measureText(currentInput).width; - // Account for prompt width and input padding/border const promptWidth = prompt.offsetWidth; const inputPadding = parseInt(computedStyle.paddingLeft || '0'); const inputBorder = parseInt(computedStyle.borderLeftWidth || '0'); - - // Add spacing between typed text and suggestion (in pixels) const suggestionSpacing = 20; - // Position suggestion overlay after the input text with spacing - // Add prompt width, input padding/border, the text width, and extra spacing this.suggestionOverlay.style.left = `${promptWidth + inputPadding + inputBorder + textWidth + suggestionSpacing}px`; } } } - /** - * Hide suggestion overlay - */ private hideSuggestion(): void { if (this.suggestionOverlay) { this.suggestionOverlay.style.display = 'none'; @@ -596,9 +570,6 @@ export class Console { this.argHint.style.display = 'flex'; } - /** - * Hide the argument-signature hint row. - */ private hideArgHint(): void { if (this.argHint) { this.argHint.style.display = 'none'; From c7fb07a00c8864db04982568d43bace14f32bd15 Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:14:46 +0200 Subject: [PATCH 3/5] fix(log-stream): render split lines whole and ignore stale poll responses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tail endpoint returns raw bytes, so a poll can land mid-line and the remainder arrives in the next chunk — one log line rendered as two. The trailing fragment is now buffered and re-rendered in place as later chunks extend it. Two related defects on the same path: - An in-flight fetch that resolved after stopStreaming() or a switch to another file appended the old stream's content and clobbered the offset. A generation counter now discards those responses. - A backwards offset means the file was truncated or rewritten, so the display is replaced instead of appending the new content under the old. - Ctrl+F only opened search on lowercase 'f' (missed with Shift/CapsLock). Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/ui/LogStreamManager.test.ts | 106 +++++++++++++++++++++++ frontend/src/ui/LogStreamManager.ts | 63 ++++++++++++-- 2 files changed, 160 insertions(+), 9 deletions(-) diff --git a/frontend/src/ui/LogStreamManager.test.ts b/frontend/src/ui/LogStreamManager.test.ts index 00ed18f..7af3ddd 100644 --- a/frontend/src/ui/LogStreamManager.test.ts +++ b/frontend/src/ui/LogStreamManager.test.ts @@ -110,3 +110,109 @@ describe('LogStreamManager search highlighting', () => { expect((document.getElementById('log-search-input') as HTMLInputElement).value).toBe(''); }); }); + +interface StreamResponse { + success: boolean; + content: string; + offset: number; + total_size: number; + filename: string; + error?: string; +} + +function streamResponse(content: string, offset: number): StreamResponse { + return { success: true, content, offset, total_size: offset, filename: 't.log' }; +} + +function renderedLines(): string[] { + return Array.from(document.querySelectorAll('.log-stream-line')) + .map(el => el.textContent ?? ''); +} + +describe('LogStreamManager streaming line assembly', () => { + // Queue of pending responses; each fetch call consumes one. + let responses: Array>; + + function queueResponse(response: StreamResponse): void { + responses.push(Promise.resolve(response)); + } + + beforeEach(() => { + vi.resetModules(); // fresh singleton per test + vi.useFakeTimers(); + responses = []; + vi.stubGlobal('fetch', vi.fn(() => { + const next = responses.shift() ?? Promise.resolve(streamResponse('', 0)); + return next.then(result => ({ json: async () => result })); + })); + buildDom([]); + }); + + afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); + document.body.innerHTML = ''; + vi.restoreAllMocks(); + }); + + it('holds a chunk\'s trailing mid-line fragment in one element and completes it in place', async () => { + const { logStreamManager } = await import('./LogStreamManager'); + + // Initial chunk ends mid-line: the writer had not finished "bravo". + queueResponse(streamResponse('alpha\nbra', 9)); + await logStreamManager.startStreaming('t.log'); + expect(renderedLines()).toEqual(['alpha', 'bra']); + + // The next poll delivers the rest of the line plus a complete one. + queueResponse(streamResponse('vo\ncharlie\n', 20)); + await vi.advanceTimersByTimeAsync(2000); + expect(renderedLines()).toEqual(['alpha', 'bravo', 'charlie']); + + logStreamManager.stopStreaming(); + }); + + it('does not render phantom empty lines for newline-terminated chunks', async () => { + const { logStreamManager } = await import('./LogStreamManager'); + + queueResponse(streamResponse('one\ntwo\n', 8)); + await logStreamManager.startStreaming('t.log'); + expect(renderedLines()).toEqual(['one', 'two']); + + queueResponse(streamResponse('three\n', 14)); + await vi.advanceTimersByTimeAsync(2000); + expect(renderedLines()).toEqual(['one', 'two', 'three']); + + logStreamManager.stopStreaming(); + }); + + it('discards a late response that arrives after the stream was stopped', async () => { + const { logStreamManager } = await import('./LogStreamManager'); + + let resolveLate: (r: StreamResponse) => void; + responses.push(new Promise(resolve => { resolveLate = resolve; })); + + const started = logStreamManager.startStreaming('t.log'); + logStreamManager.stopStreaming(); + + resolveLate!(streamResponse('stale\n', 6)); + await started; + + expect(renderedLines()).toEqual([]); + }); + + it('replaces the display when the server restarts a truncated file with a fresh tail', async () => { + const { logStreamManager } = await import('./LogStreamManager'); + + queueResponse(streamResponse('old1\nold2\n', 10)); + await logStreamManager.startStreaming('t.log'); + expect(renderedLines()).toEqual(['old1', 'old2']); + + // The file shrank (re-run scenario logging to the same name): the + // server reset to tail mode and returned an offset below ours. + queueResponse(streamResponse('new\n', 4)); + await vi.advanceTimersByTimeAsync(2000); + expect(renderedLines()).toEqual(['new']); + + logStreamManager.stopStreaming(); + }); +}); diff --git a/frontend/src/ui/LogStreamManager.ts b/frontend/src/ui/LogStreamManager.ts index 6325a20..85d2a6a 100644 --- a/frontend/src/ui/LogStreamManager.ts +++ b/frontend/src/ui/LogStreamManager.ts @@ -37,6 +37,13 @@ export class LogStreamManager { private maxLines: number = 1000; private pollIntervalMs: number = 2000; private isInitialized = false; + // Bumped on every stop/switch so responses of in-flight fetches from the + // previous stream are discarded instead of corrupting the new one. + private streamGeneration = 0; + // Trailing chunk fragment not yet terminated by a newline, and the + // element rendering it (extended in place by later chunks). + private partialLine: string = ''; + private partialLineEl: HTMLElement | null = null; // Search state private searchMatches: HTMLElement[] = []; @@ -98,6 +105,9 @@ export class LogStreamManager { if (this.clearStreamBtn) { this.clearStreamBtn.addEventListener('click', () => { if (this.logStreamOutput) this.logStreamOutput.innerHTML = ''; + // Keep the partial-line buffer (the line is still being + // written) but drop the now-detached element rendering it. + this.partialLineEl = null; this.clearSearch(); }); } @@ -137,7 +147,7 @@ export class LogStreamManager { // Ctrl+F to open search when streaming document.addEventListener('keydown', (e) => { - if ((e.ctrlKey || e.metaKey) && e.key === 'f') { + if ((e.ctrlKey || e.metaKey) && e.key.toLowerCase() === 'f') { if (this.isStreaming && this.logStreamTabBtn?.classList.contains('active')) { e.preventDefault(); this.openSearch(); @@ -156,6 +166,8 @@ export class LogStreamManager { if (this.logStreamOutput) { this.logStreamOutput.innerHTML = ''; } + this.partialLine = ''; + this.partialLineEl = null; this.showStreamView(); this.updateControls(); @@ -170,6 +182,7 @@ export class LogStreamManager { } public stopStreaming(): void { + this.streamGeneration++; if (this.pollingInterval) { clearInterval(this.pollingInterval); this.pollingInterval = null; @@ -184,6 +197,7 @@ export class LogStreamManager { private async fetchContent(isInitial: boolean): Promise { if (!this.currentFilepath) return; + const generation = this.streamGeneration; try { const encodedPath = encodeURIComponent(this.currentFilepath); @@ -198,10 +212,20 @@ export class LogStreamManager { const response = await fetch(url); const result: StreamContentResponse = await response.json(); + // The stream was stopped or switched to another file while this + // request was in flight; its content and offset belong to the + // previous stream. + if (generation !== this.streamGeneration) return; + if (result.success) { + // An offset moving backwards means the file was truncated or + // rewritten and the server restarted with a fresh tail, so + // replace the display instead of appending the new file's + // content below the old one. + const replace = isInitial || result.offset < this.currentOffset; this.currentOffset = result.offset; - if (result.content) { - this.appendContent(result.content, isInitial); + if (result.content || (replace && !isInitial)) { + this.appendContent(result.content, replace); } } else { logger.warn('LogStreamManager', `Stream error: ${result.error}`); @@ -219,16 +243,30 @@ export class LogStreamManager { if (replace) { this.logStreamOutput.innerHTML = ''; + this.partialLine = ''; + this.partialLineEl = null; } - const lines = text.split('\n'); - const fragment = document.createDocumentFragment(); + // The server returns raw bytes, so a chunk can end mid-line (the + // writer had only flushed part of it when the poll landed). Render + // complete lines as fixed elements and keep the trailing fragment in + // a single "partial" element that is replaced when later chunks + // extend or complete it, so one log line never renders as two. + const lines = (this.partialLine + text).split('\n'); + this.partialLine = lines.pop() ?? ''; + + if (this.partialLineEl) { + this.partialLineEl.remove(); + this.partialLineEl = null; + } + const fragment = document.createDocumentFragment(); for (const line of lines) { - const lineEl = document.createElement('div'); - lineEl.className = 'log-stream-line'; - lineEl.textContent = line; - fragment.appendChild(lineEl); + fragment.appendChild(this.createLineElement(line)); + } + if (this.partialLine) { + this.partialLineEl = this.createLineElement(this.partialLine); + fragment.appendChild(this.partialLineEl); } this.logStreamOutput.appendChild(fragment); @@ -236,6 +274,13 @@ export class LogStreamManager { this.logStreamOutput.scrollTop = this.logStreamOutput.scrollHeight; } + private createLineElement(text: string): HTMLElement { + const lineEl = document.createElement('div'); + lineEl.className = 'log-stream-line'; + lineEl.textContent = text; + return lineEl; + } + private limitLines(): void { if (!this.logStreamOutput) return; From 82bdce95cefc7d56fd1ecc4530542237ff949924 Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:14:58 +0200 Subject: [PATCH 4/5] fix(aircraft-3d): fall back to the default model when a GLB fails to load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A model that 404s or fails to parse left its aircraft queued in pendingAircraft forever — invisible in 3D — and the loader re-requested the broken path on every data tick, since only loadingModels was cleared on error. The loader now records failed paths, skips re-requesting them, and reports the failure. Aircraft3DFleet.redirectPending() re-creates the stranded aircraft with the fallback model (queueing again if it hasn't loaded yet) or drops them with a warning when there is no fallback. Aircraft3DCustomLayer also substitutes the fallback at resolve time, so the path stays stable across ticks and the mesh isn't rebuilt. Failures are recorded under the same teardown guard as the success callback and cleared with the cache, so a model reload retries them. Co-Authored-By: Claude Opus 5 (1M context) --- .../ui/map/aircraft/Aircraft3DCustomLayer.ts | 47 ++++++----- .../ui/map/aircraft/Aircraft3DFleet.test.ts | 84 +++++++++++++++++++ .../src/ui/map/aircraft/Aircraft3DFleet.ts | 24 ++++++ .../aircraft/Aircraft3DModelLoader.test.ts | 83 +++++++++++++++++- .../ui/map/aircraft/Aircraft3DModelLoader.ts | 26 +++++- 5 files changed, 237 insertions(+), 27 deletions(-) diff --git a/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts index 744907e..1632b65 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DCustomLayer.ts @@ -32,7 +32,8 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { // Default/fallback model path — used when aircraft type is unknown or missing. private modelPath: string = `${MODEL_DIR}${DEFAULT_FALLBACK_MODEL}`; private pendingAircraftData: AircraftData | null = null; - private lastProjectionMode: boolean | null = null; // Track projection mode changes for debug logging + // Last seen projection mode, to detect globe/mercator switches. + private lastProjectionMode: boolean | null = null; // Wall-clock delta source for GLB animation playback (engines, rotors). private readonly animationClock = new THREE.Clock(); @@ -52,7 +53,11 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { this.stateManager = stateManager; this.modelLoader = new Aircraft3DModelLoader({ getMaxAnisotropy: () => this.renderer?.capabilities?.getMaxAnisotropy?.() || 16, - onModelLoaded: (path) => this.fleet.processPending(path) + onModelLoaded: (path) => this.fleet.processPending(path), + onModelFailed: (path) => { + const fallback = this.usableModelPath(path); + this.fleet.redirectPending(path, fallback !== path ? fallback : null); + }, }); this.transforms = new Aircraft3DTransforms({ getMap: () => this.map ?? null, @@ -140,7 +145,12 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { const actype = aircraftData.actype?.[i] ?? ''; const override = this.stateManager?.getAircraftModelOverride(id) ?? null; - const modelPath = resolveAircraftModelPath(selectedModel, actype, override); + // Substitute the fallback model when the resolved one is known to + // fail. Doing it here (not just in the failure callback) keeps + // the path stable across ticks, so the mesh isn't rebuilt. + const modelPath = this.usableModelPath( + resolveAircraftModelPath(selectedModel, actype, override) + ); const data: AircraftMeshData = { lat: aircraftData.lat[i], @@ -179,10 +189,20 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { } }); - // Also remove from pending aircraft if they no longer exist this.fleet.prunePending(activeIds); } + /** + * The given model path if it is expected to load, otherwise the default + * fallback model — unless that failed too, in which case the original + * path is returned and the aircraft stays queued (the loader won't + * re-request a failed path, so this stays cheap). + */ + private usableModelPath(path: string): string { + if (!this.modelLoader.hasFailed(path)) return path; + return this.modelLoader.hasFailed(this.modelPath) ? path : this.modelPath; + } + /** * React to changes in the per-aircraft model override map. Rebuilds * the mesh for any aircraft whose override value changed. @@ -204,11 +224,11 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { if (!existing) return; const overrideFile = newOverrides[id]; - const resolvedPath = resolveAircraftModelPath( + const resolvedPath = this.usableModelPath(resolveAircraftModelPath( this.displayOptions.selectedAircraftModel, existing.data.actype, overrideFile, - ); + )); if (resolvedPath === existing.modelPath) return; @@ -235,15 +255,11 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { // repaint after every render, so playback is continuous. this.fleet.advanceAnimations(this.animationClock.getDelta()); - // Check if we're in globe projection mode const isGlobe = this.isGlobeProjection(); - // Debug log projection mode changes and handle mesh group transitions if (this.lastProjectionMode !== isGlobe) { logger.info('Aircraft3DCustomLayer', `[PROJECTION] Switched to ${isGlobe ? 'GLOBE' : 'MERCATOR'} mode`); this.lastProjectionMode = isGlobe; - - // Move all aircraft to the appropriate group for the new projection mode this.fleet.switchGroups(isGlobe); // Re-aim the shared directional lights for the active group's @@ -251,7 +267,6 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { this.updateLightsForProjection(isGlobe); } - // Toggle visibility of groups based on projection mode if (this.globeGroup) this.globeGroup.visible = isGlobe; if (this.mercatorGroup) this.mercatorGroup.visible = !isGlobe; @@ -259,11 +274,8 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { if (args) { this.transforms.applyGlobeCamera(args); } - - // Update all aircraft transforms for globe positioning this.fleet.applyGlobeTransforms(); } else if (args && this.transforms.applyMercatorCamera(args)) { - // Make sure all aircraft use relative positioning (matrixAutoUpdate = true) this.fleet.enableMatrixAutoUpdate(); } } @@ -320,13 +332,10 @@ export class Aircraft3DCustomLayer extends CustomLayer3D { logger.info('Aircraft3DCustomLayer', `Default aircraft model changed to: ${this.modelPath}`); - // Remove all existing meshes this.fleet.removeAllForReload(); - - // Drop cached models so the new fallback (and any stale asset) reloads fresh + // Drop cached models (and recorded load failures) so the new + // fallback and any stale asset reload fresh. this.modelLoader.clearCache(); - - // Preload the new default this.modelLoader.load(this.modelPath); } diff --git a/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts index 224879c..bc407fa 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DFleet.test.ts @@ -117,6 +117,90 @@ describe('Aircraft3DFleet lifecycle', () => { }); }); +describe('Aircraft3DFleet pending-model fallback', () => { + beforeEach(() => vi.restoreAllMocks()); + + /** Fleet whose model loader only has models for the given paths. */ + function makeFleetWithModels(models: Record) { + const mercatorGroup = new THREE.Group(); + const load = vi.fn(); + const deps: Aircraft3DFleetDeps = { + modelLoader: { + get: (path: string) => models[path], + load, + rawMaxDim: () => 10, + animations: () => [], + } as unknown as Aircraft3DFleetDeps['modelLoader'], + transforms: { + updateMeshTransform: vi.fn(), + updateMeshTransformForGlobe: vi.fn(), + } as unknown as Aircraft3DFleetDeps['transforms'], + getMercatorGroup: () => mercatorGroup, + getGlobeGroup: () => null, + isGlobeProjection: () => false, + }; + return { fleet: new Aircraft3DFleet(deps), mercatorGroup, load }; + } + + it('redirectPending creates queued aircraft with the fallback model', () => { + const { model } = multiMaterialModel(); + const { fleet, mercatorGroup } = makeFleetWithModels({ 'A320.glb': model }); + + fleet.create('AC1', DATA, 'missing.glb'); // queued, model not available + expect(fleet.size).toBe(0); + + fleet.redirectPending('missing.glb', 'A320.glb'); + + expect(fleet.size).toBe(1); + expect(fleet.get('AC1')?.modelPath).toBe('A320.glb'); + expect(mercatorGroup.children.length).toBe(1); + }); + + it('redirectPending re-queues when the fallback model is not loaded yet', () => { + const models: Record = {}; + const { fleet, load } = makeFleetWithModels(models); + + fleet.create('AC1', DATA, 'missing.glb'); + fleet.redirectPending('missing.glb', 'fallback.glb'); + + expect(fleet.size).toBe(0); + expect(load).toHaveBeenCalledWith('fallback.glb'); + + // The fallback finishes loading -> the aircraft is created with it. + models['fallback.glb'] = multiMaterialModel().model; + fleet.processPending('fallback.glb'); + + expect(fleet.get('AC1')?.modelPath).toBe('fallback.glb'); + }); + + it('redirectPending without a fallback drops the queued aircraft', () => { + const { model } = multiMaterialModel(); + const { fleet, mercatorGroup } = makeFleetWithModels({ 'A320.glb': model }); + + fleet.create('AC1', DATA, 'missing.glb'); + fleet.redirectPending('missing.glb', null); + + expect(fleet.size).toBe(0); + expect(mercatorGroup.children.length).toBe(0); + }); + + it('redirectPending leaves aircraft queued for other models alone', () => { + const models: Record = {}; + const { fleet } = makeFleetWithModels(models); + + fleet.create('AC1', DATA, 'missing.glb'); + fleet.create('AC2', DATA, 'other.glb'); + fleet.redirectPending('missing.glb', null); + + // AC2 is still queued: when its model arrives it is created. + models['other.glb'] = multiMaterialModel().model; + fleet.processPending('other.glb'); + + expect(fleet.get('AC1')).toBeUndefined(); + expect(fleet.get('AC2')?.modelPath).toBe('other.glb'); + }); +}); + describe('Aircraft3DFleet GLB animation playback', () => { beforeEach(() => vi.restoreAllMocks()); diff --git a/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts b/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts index b56ea0d..5950855 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DFleet.ts @@ -242,6 +242,30 @@ export class Aircraft3DFleet { } } + /** + * Re-queue aircraft waiting on a model that failed to load. With a + * fallback path they are created with that model instead (queueing + * again if it isn't loaded yet); without one they are dropped and + * stay absent from the 3D scene. + */ + redirectPending(fromPath: string, toPath: string | null): void { + const stranded: Array<{ id: string; data: AircraftMeshData }> = []; + this.pendingAircraft.forEach((entry, id) => { + if (entry.modelPath === fromPath) { + stranded.push({ id, data: entry.data }); + } + }); + + for (const { id, data } of stranded) { + this.pendingAircraft.delete(id); + if (toPath) { + this.create(id, data, toPath); + } else { + logger.warn('Aircraft3DFleet', `No fallback model for aircraft ${id}; not rendered in 3D`); + } + } + } + /** * Re-apply the projection-appropriate transform to every aircraft mesh. */ diff --git a/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.test.ts b/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.test.ts index 38141fd..36c3e83 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.test.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.test.ts @@ -6,14 +6,25 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import * as THREE from 'three'; -// Capture the GLTFLoader onLoad callback so tests can drive a "loaded" model -// through the loader without touching the network. +// Capture the GLTFLoader callbacks so tests can drive a load to success or +// failure without touching the network, and count the requests issued. type MockGltf = { scene: THREE.Group; animations?: THREE.AnimationClip[] }; -const captured: { onLoad?: (gltf: MockGltf) => void } = {}; +const captured: { + onLoad?: (gltf: MockGltf) => void; + onError?: (error: unknown) => void; + loadCalls: string[]; +} = { loadCalls: [] }; vi.mock('three/addons/loaders/GLTFLoader.js', () => ({ GLTFLoader: class { - load(_path: string, onLoad: (gltf: MockGltf) => void) { + load( + path: string, + onLoad: (gltf: MockGltf) => void, + _onProgress?: unknown, + onError?: (error: unknown) => void + ) { + captured.loadCalls.push(path); captured.onLoad = onLoad; + captured.onError = onError; } }, })); @@ -168,3 +179,67 @@ describe('Aircraft3DModelLoader animation clips', () => { expect(loader.animations('prop.glb')).toEqual([]); }); }); + +describe('Aircraft3DModelLoader load failures', () => { + beforeEach(() => { + vi.restoreAllMocks(); + captured.onLoad = undefined; + captured.onError = undefined; + captured.loadCalls = []; + }); + + function makeFailureLoader() { + const onModelFailed = vi.fn(); + const loader = new Aircraft3DModelLoader({ + getMaxAnisotropy: () => 1, + onModelLoaded: vi.fn(), + onModelFailed, + }); + return { loader, onModelFailed }; + } + + it('records a failed load and notifies onModelFailed', () => { + const { loader, onModelFailed } = makeFailureLoader(); + + loader.load('missing.glb'); + captured.onError?.(new Error('404')); + + expect(loader.hasFailed('missing.glb')).toBe(true); + expect(onModelFailed).toHaveBeenCalledWith('missing.glb'); + }); + + it('does not re-request a path that already failed', () => { + const { loader } = makeFailureLoader(); + + loader.load('missing.glb'); + captured.onError?.(new Error('404')); + loader.load('missing.glb'); + loader.load('missing.glb'); + + expect(captured.loadCalls).toEqual(['missing.glb']); + }); + + it('clearCache() forgets failures so a reload retries the path', () => { + const { loader } = makeFailureLoader(); + + loader.load('missing.glb'); + captured.onError?.(new Error('404')); + loader.clearCache(); + loader.load('missing.glb'); + + expect(loader.hasFailed('missing.glb')).toBe(false); + expect(captured.loadCalls).toEqual(['missing.glb', 'missing.glb']); + }); + + it('ignores a failure that arrives after clearAll()', () => { + const { loader, onModelFailed } = makeFailureLoader(); + + loader.load('missing.glb'); + loader.clearAll(); // teardown while the load is in flight + + captured.onError?.(new Error('404')); + + expect(loader.hasFailed('missing.glb')).toBe(false); + expect(onModelFailed).not.toHaveBeenCalled(); + }); +}); diff --git a/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.ts b/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.ts index 9934e54..94459dd 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DModelLoader.ts @@ -24,12 +24,18 @@ export class Aircraft3DModelLoader { // its own AnimationMixer. private readonly animationClips = new Map(); private readonly loadingModels = new Set(); + // Paths whose load failed (404, parse error, ...). load() skips these so + // a broken model isn't re-requested on every data tick; cleared together + // with the cache so a model reload retries them. + private readonly failedModels = new Set(); constructor(private readonly opts: { /** Renderer texture-anisotropy limit, queried at normalize time. */ getMaxAnisotropy: () => number; /** Called after a model finishes loading and is cached. */ onModelLoaded: (path: string) => void; + /** Called when a model load fails, so queued aircraft can fall back. */ + onModelFailed?: (path: string) => void; }) {} /** Cached model for a path, when loaded. */ @@ -47,13 +53,18 @@ export class Aircraft3DModelLoader { return this.animationClips.get(path) ?? []; } + /** Whether a previous load of this path failed (and wasn't retried). */ + public hasFailed(path: string): boolean { + return this.failedModels.has(path); + } + /** * Load a GLTF/GLB model by URL, caching it for future clones. - * Idempotent: returns immediately if the model is already loaded - * or currently loading. + * Idempotent: returns immediately if the model is already loaded, + * currently loading, or known to fail. */ public load(path: string): void { - if (this.loadedModels.has(path) || this.loadingModels.has(path)) { + if (this.loadedModels.has(path) || this.loadingModels.has(path) || this.failedModels.has(path)) { return; } @@ -87,8 +98,14 @@ export class Aircraft3DModelLoader { } }, (error) => { - this.loadingModels.delete(path); logger.error('Aircraft3DModelLoader', `Failed to load aircraft model ${path}: ${error}`); + // Same teardown guard as the success callback: after + // clearAll() the owner is gone, so don't record the failure + // or notify — a fresh owner starts with a clean slate. + if (!this.loadingModels.has(path)) return; + this.loadingModels.delete(path); + this.failedModels.add(path); + this.opts.onModelFailed?.(path); } ); } @@ -103,6 +120,7 @@ export class Aircraft3DModelLoader { this.loadedModels.clear(); this.rawMaxDims.clear(); this.animationClips.clear(); + this.failedModels.clear(); } /** From 77284a985db0f3555c45df563bedcf1251a799ba Mon Sep 17 00:00:00 2001 From: Andres Morfin Veytia <78442543+amorfinv@users.noreply.github.com> Date: Sat, 15 Aug 2026 08:14:58 +0200 Subject: [PATCH 5/5] chore: drop comments that restate the code Removes docblocks and inline comments that only repeat the method name ("Initialize header component", "Set scale") in the files touched by the preceding fixes, and inlines a single-use matrix variable in Aircraft3DTransforms. Co-Authored-By: Claude Opus 5 (1M context) --- frontend/src/core/App.ts | 42 ------------------- .../ui/map/aircraft/Aircraft3DTransforms.ts | 12 +----- 2 files changed, 2 insertions(+), 52 deletions(-) diff --git a/frontend/src/core/App.ts b/frontend/src/core/App.ts index ba3c380..6eb8dda 100644 --- a/frontend/src/core/App.ts +++ b/frontend/src/core/App.ts @@ -113,17 +113,11 @@ export class App { } } - /** - * Initialize application state - */ private initializeState(): void { this.setupConnectionStatusHandlers(); this.setupSimulationDataHandlers(); } - /** - * Set up handlers for connection status changes - */ private setupConnectionStatusHandlers(): void { // ConnectionStatusService is the single source of truth for all // connection state. @@ -141,9 +135,6 @@ export class App { // (handled in checkInitialConnectionStatus method). } - /** - * Set up handlers for simulation data changes - */ private setupSimulationDataHandlers(): void { // Header shows simulation time, rate, etc. this.stateManager.subscribe('simInfo', (newSimInfo) => { @@ -194,10 +185,6 @@ export class App { }); } - /** - * Initialize UI components - * Sets up UI modules and their event handlers - */ private initializeUI(): void { this.initializeHeader(); this.initializeConsole(); @@ -232,9 +219,6 @@ export class App { }); } - /** - * Initialize header component - */ private initializeHeader(): void { this.header.init(); this.header.setSocketManager(this.socketManager); @@ -242,9 +226,6 @@ export class App { logger.debug('App', 'Header initialized'); } - /** - * Initialize console component - */ private initializeConsole(): void { this.console.setStateManager(this.stateManager); this.console.setCommandHandler(this.commandHandler); @@ -254,9 +235,6 @@ export class App { logger.debug('App', 'Console component initialized'); } - /** - * Initialize control panels - */ private initializeControlPanels(): void { this.simulationNodesPanel.init(); this.simulationNodesPanel.setSocketManager(this.socketManager); @@ -305,9 +283,6 @@ export class App { return el; } - /** - * Initialize map display - */ private initializeMapDisplay(): void { this.mapDisplay.initialize(); this.mapDisplay.setupStyleSelector(); @@ -402,26 +377,18 @@ export class App { logger.debug('App', 'Map display and interaction managers initialized'); } - /** - * Initialize modal dialogs - */ private initializeModals(): void { modals.forceInitialize(); logger.debug('App', 'Modal system initialized'); } - /** - * Set up global event listeners - */ private setupGlobalEventListeners(): void { const { signal } = this.globalListenerAbort; - // Handle before page unload window.addEventListener('beforeunload', () => { this.cleanup(); }, { signal }); - // Handle resize events window.addEventListener('resize', () => { this.handleResize(); }, { signal }); @@ -479,9 +446,6 @@ export class App { }); } - /** - * Handle window resize - */ private handleResize(): void { if (this.mapDisplay && this.mapDisplay.isInitialized()) { this.mapDisplay.resize(); @@ -501,9 +465,6 @@ export class App { } } - /** - * Send command to simulation - */ public sendCommand(command: string): Promise { return this.socketManager.sendCommand(command); } @@ -527,9 +488,6 @@ export class App { return this.aircraftInteractionManager; } - /** - * Set active simulation node - */ public setActiveNode(nodeId: string): void { this.socketManager.setActiveNode(nodeId); this.stateManager.setActiveNode(nodeId); diff --git a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts index b5fa640..e3c1878 100644 --- a/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts +++ b/frontend/src/ui/map/aircraft/Aircraft3DTransforms.ts @@ -211,7 +211,6 @@ export class Aircraft3DTransforms { headingRad: number; finalScale: number; } { - // Altitude already in meters from BlueSky const altitudeMeters = data.alt; const headingRad = THREE.MathUtils.degToRad(data.hdg); const realScale = this.getMeshRealScale(mesh); @@ -249,13 +248,9 @@ export class Aircraft3DTransforms { // Heading is a Y (vertical-axis) rotation: aviation 0°=N maps to // three.js +Z, minus π/2 for the model's default orientation. mesh.rotation.set(0, headingRad - Math.PI / 2, 0); - - // Set scale mesh.scale.set(finalScale, finalScale, finalScale); this.disableFrustumCulling(mesh); - - // Enable automatic matrix updates for this positioning approach mesh.matrixAutoUpdate = true; } @@ -316,7 +311,7 @@ export class Aircraft3DTransforms { // Same heading convention as updateMeshTransform, folded into an // absolute mercator matrix instead of scene-relative position. - const transformMatrix = this.deps.createFallbackMatrix( + mesh.matrix = this.deps.createFallbackMatrix( data.lon, data.lat, altitudeMeters, @@ -325,10 +320,7 @@ export class Aircraft3DTransforms { 0, // roll finalScale ); - - // Apply the transform matrix directly - mesh.matrix = transformMatrix; - mesh.matrixAutoUpdate = false; // We're manually setting the matrix + mesh.matrixAutoUpdate = false; this.disableFrustumCulling(mesh); }