TUI: match ui_kits/tui concept across every page#15
Merged
Conversation
Reworks the Textual TUI to mirror the design bundle at `design/ui_kits/tui/index.html` on every surface: - Topbar: brand · db · flights · aircraft · traces · job · UTC clock, dot-separated, right-aligned trailing. `count_trace_bytes` backs the traces field. - Sidebar: tinted active row + cyan left marker + page-bg kbd pill, Views / Operations / Session groups. - ActionBar replaces Textual's stock Footer with a single-row kbd hint strip and trailing `<page> · normal` mode indicator. - PageHeader: dot-separated crumb with right-aligned trailing detail, consistent across all seven views. - Aircraft: TYPE dim, cyan `last_seen ↓` sort indicator, `(unknown)` fallback, dashed flags. - Flights: TYPE column replaces LAND; airport codes colour-coded (ok / amber / red / dim) per concept; mission magenta; header uses dot separators. - Events: outlined pills, TYPE column dropped (the pill is the type), severity counts rendered as trailing pills. - Spoof: PER-SOURCE column built from reason_detail.source_rates; inline detail panel with violet left border. - Map: braille canvas plus HUD strips (layers + cursor info on top, scalebar + scrubber on bottom) mirroring the SVG concept. - Status: full card-grid rebuild with 4 stat cards, weighted position-source bars, mission-mix bars, indicators, signal tier + 52-week sparkline, FAA registry card. `status_snapshot` gains the derived indicator counts. - Ops: 2-col card grid with progress bars parsed from stdout. - New JumpToHex (`:`) and Help (`?`) modal screens. Smoke tests (`tests/test_tui_app.py`) spin the full app headless and cycle the view bindings so future regressions are caught by CI. https://claude.ai/code/session_01MW6KrntUD9LTXQwArjKYCd
Branch landed with 13 ruff errors and 3 failing tests (test_tui_app's async tests couldn't run because pytest-asyncio was missing from the dev extras that CI installs via `uv sync --extra dev`). - Add pytest-asyncio to project.optional-dependencies.dev (the group CI uses). Drop the stray [dependency-groups] entry `uv add --dev` wrote earlier so deps live in one place. - Declare asyncio_mode = "strict" under pytest so the explicit module pytestmark in test_tui_app stays load-bearing. - Split three long SQL CASE expressions in status_snapshot across lines to stay under the 120-col ruff limit. - Split the long keyboard-shortcut row in jump.HelpScreen the same way. - Split the ops job hint string: one f-string per kbd hint. - Apply the SIM108 ternary suggestion on status._bar_row and map.MapTraceInfoStrip.set_point. - Drop the unused `total` local in status._build_missions_body (it was computed but never read; `top` drives the bar ratio instead). All 545 tests pass, ruff check + format --check both clean.
…n tests) Five-agent review on PR #15 surfaced three critical issues, six important issues, and four in-scope polish items. Type-design refactors (Hex24 newtype, StatusSnapshot dataclass, DataTable row-key pattern) were explicitly deferred to a follow-up PR. Critical: - app.on_mount: replace the blanket contextlib.suppress(Exception) that wrapped all three DB counts with per-call sqlite3.DatabaseError handling that surfaces the error via self.notify. A schema mismatch or migration regression previously rendered 0 flights / 0 aircraft / 0 B silently; now the user sees the exception. - status._build_missions_body: drop the dead first-pass loop that built rows via a brittle _bar_row().replace() hack and was immediately thrown away by the rebuild below it. One loop, no string surgery. - app.tcss: delete the #map-body / MapCanvas / .map-hud / .map-layers / .map-info / .map-scalebar / .map-scrubber rules. They referenced an ID and classes the MapView compose tree never produced; the map rendered by accident of each HUD widget's inline DEFAULT_CSS. Layout is now unambiguously the Vertical / _HudRow / canvas / _HudRow tree documented in map.py. Important: - widgets._widget_width: narrow bare except Exception to (RuntimeError, NoMatches). RuntimeError is what Textual raises from DOM resolution when the widget is not yet mounted; NoMatches covers the detached-tree path. A broken widget.size API would now surface as AttributeError instead of silently returning a 120-col fallback. - tests/test_tui_app.py: the original smoke test pressed 5/6 on an empty DB, which short-circuited via the _current_icao guard and never touched FlightsView / MapView / StatusView set_icao paths. Add test_app_navigates_after_selecting_aircraft that opens the seeded ccc333 and cycles every view with switcher.current assertions. - tests/test_tui_queries.py: the previous indicator test only asserted zeros. Add per-branch coverage for each SUM(CASE WHEN ...) in status_snapshot (emergency, go_around, long_hover, signal_lost, off_airport, confirmed) plus a test_status_snapshot_unknown_icao that pins the None-stats shape. - map.MapTraceInfoStrip / MapScrubberStrip: docstrings claimed a "trace cursor" and "playback scrubber" that the code does not implement (the strips only ever render the last point and a full- span progress bar). Rewrite to match behaviour; rename the "CURSOR" label to "LAST"; "(no cursor)" fallback becomes "(no trace)". - jump.action_accept: replace silent dismiss(None) on empty-match Enter with a user-visible notification naming the query. - widgets.pill_markup: docstring claimed an "outlined" pill with a simulated border; the implementation is plain tinted-bg coloured text. Rewrite honestly. Suggestions: - ops._render_job: drop the cmd_tail conditional that stripped a ['uv', 'run', 'python', '-m'] prefix that _launch never added. - status._build_signal_body: hoist the mid-function import math to module scope; relabel "weekly uptime, 52 weeks" to "activity strip (placeholder, not real uptime)" so the label no longer overstates what the deterministic sparkline is showing. - status._build_sources_body: drop the hardcoded ADS-R 0.0 row (queries never populated it) and add an OTHER row backed by the already-computed sources['other']. The card no longer misrepresents coverage. - widgets.Sidebar: delete the orphaned _ROW_WIDTH constant and the three-line comment above _LABEL_WIDTH that restated the self- documenting code. Tests: tests/conftest.py now hosts the seeded_db fixture so the TUI app smoke tests and query tests share one definition. All 548 tests pass, ruff check and format --check clean, mypy clean on the touched TUI files.
…rings Second review pass on PR #15 flagged two important + five suggestion items after the first fix commit. Addressing all of them in one go. Important: - tests/test_tui_app.py: the nav test previously only asserted switcher.current. A regression where set_icao silently swallowed an exception and rendered empty widgets would still pass. Now the test also asserts FlightsView's DataTable has the one seeded row, Sidebar._active tracks the current view after each key press, and the rendered StatusView grid contains the seeded "N111AA" from aircraft_registry (so a regression in the registry-merge path in status_snapshot would fail the test). - adsbtrack/tui/views/map.py: MapScrubberStrip docstring tail ("reserved for a future scrubber") was the same aspirational-rot profile we just cleaned up on the other map widgets. Drop the forward-looking clause; keep the behavioural description. Suggestions: - adsbtrack/tui/widgets.py: the new _widget_width except-block comment was restating the narrowed exception tuple; the function docstring already explains the pre-mount "why". Delete the redundant comment. - adsbtrack/tui/views/map.py: MapLayersStrip / MapTraceInfoStrip / MapScalebarStrip / MapScrubberStrip no longer carry classes="map-*" kwargs since the matching CSS rules were removed last commit - the orphan class attributes were noise for future greps. - tests/test_tui_queries.py: add three tests that pin behaviour the prior suite didn't cover: * days_with_data correctly counts distinct trace_days dates (including across multiple sources that share a date). * sources weighted-average math: a 900-point ADS-B-heavy flight and a 100-point MLAT-heavy flight weight correctly (82%/18%). * missions null-filter drops None mission_type and the LIMIT 6 clause holds against seven distinct missions. - tests/test_tui_queries.py: add a one-line note that off_airport is a subset of confirmed landings so future readers don't "fix" the == 4 assertion down to == 3.
Third review pass on PR #15 surfaced one important item (missing cross-ICAO isolation in the days_with_data test) plus a handful of polish suggestions. Addressing them in one commit. Important: - tests/test_tui_queries.py: test_status_snapshot_days_with_data_counts_trace_days now seeds a second ICAO with a third date. A regression that drops the WHERE icao = ? predicate in queries.py would previously have passed the test; it now fails loudly. Polish: - tests/test_tui_queries.py: missions test seeds the "training" mission twice so its count is 2, pinning the ORDER BY n DESC head. Adds an assertion on snap["missions"][0] so future regressions in the order-by or LIMIT are caught instead of riding on SQLite's unspecified tie-breaking. - tests/test_tui_queries.py: new test_status_snapshot_sources_returns_none_when_all_points_zero pins the NULLIF zero-guard path in status_snapshot - every flight with data_points=0 should resolve sources to None rather than crashing or returning a zero-denominator result. - tests/test_tui_queries.py: tighten the off_airport overlap comment to the load-bearing sentence. - adsbtrack/tui/views/map.py: MapScrubberStrip docstring no longer overclaims; describes both the loaded and unloaded states. - tests/test_tui_app.py: trim two comment blocks that narrated the test flow; keep only the regression-class-explaining sentences.
CI installs only --extra dev (no textual). The tui/__init__.py eager
`from .app import AdsbtrackApp` pulled textual in at package-load time,
which broke collection of any test that imports `adsbtrack.tui.queries`
or `adsbtrack.gui_export` in a dev-only environment:
ModuleNotFoundError: No module named 'textual'
at tests/test_tui_queries.py and tests/test_gui_export.py
`adsbtrack/tui/queries.py` is pure sqlite3 and is the module
`adsbtrack/gui_export.py` depends on for its query layer; it does not
need Textual at all. The single consumer of `adsbtrack.tui.AdsbtrackApp`
(the `tui` CLI subcommand) already has an ImportError guard and now
imports from the submodule directly.
Also adds a pytest.importorskip("textual") to tests/test_tui_app.py is
already in place; the queries and gui_export tests no longer drag
textual in, so they collect successfully under --extra dev alone.
Verified: `uv sync --extra dev` + `uv run pytest` = 548 passed, 2
skipped. With tui extra: 552 passed, 1 skipped.
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
2 participants
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.
Summary
design/ui_kits/tui/index.html: new topbar, sidebar, ActionBar (replacing stock Footer), and PageHeader chrome; redesigned Aircraft / Flights / Events / Spoof / Map / Status / Ops views; new JumpToHex (:) and Help (?) modal screens.count_trace_bytesto back the traces field in the topbar, and extendstatus_snapshotwith the derived indicator counts the new status card grid needs.pytest-asynciointo thedevextra so CI actually runs the newtests/test_tui_app.pysmoke tests (they were silently skipping before). Split long SQL and kbd-hint strings to clear ruff's 120-col limit.Test plan
uv run ruff check .uv run ruff format --check .uv run pytest(545 tests pass, including the new TUI smoke tests)uv run python -m adsbtrack.cli tuiand cycle all seven views plus the JumpToHex and Help modals