feat(engine+ui): D-254 companion wave 2 — six-entity data browser over the wire #184
Assignees
jpmschweitzer (Jeroen Schweitzer)
Clear assignees
jpmschweitzer (Jeroen Schweitzer)
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: jpmschweitzer/settled-reach#184
Reference in New Issue
Block a user
Summary
Wave 2 of the Atlas companion app (D-254 §4, epic T-1128): the six-entity browse surface, end to end. After this merges,
make atlas(and later the in-game implant — it's a normal implant app) gets a data browser over star systems, bodies, stations, corporations, commodities, and trait templates, index + detail, against live server data. This is Jeroen's "lightweight app to scan all database data" deliverable.T-1131 — browse data proxy (server, Oscar)
browse_reader.rs:BrowseReaderon theCityContextReader::open()pattern — six index + six detail reads against systems.db (bodies filterable by containing system; system/corporation/commodity details fold their 1:1/1:N join partners). Wire-only per D-254 (T-949 precedent — no SQLite GDExtension); v1 exclusions (cascade-derived geometry, event logs) respected.browse_proxy.rs:BrowseRequest{browse, kind, query}/BrowseResponse{kind, status, index, detail};BrowseIndexRow{id, primary, secondary}generic across kinds;BrowseDetailper-kind enum of field-exhaustive structs.Inbound::BrowseRequestis the FIFTH hand-sniffed map shape against a documented four-shape ceiling. It stays at five because six kinds × two forms are folded into ONE envelope whose internal enums pick sub-behavior (theAtlasLayerRequest.up_toprecedent) instead of twelve top-level shapes; the doc now re-pins the ceiling at five and mandates the deferred D-225 tagged-envelope migration for any sixth shape. Reviewers: this is the architectural decision of the PR — weigh it.serve_browse_requestspub so integration tests drive the true end-to-end pipeline.T-1133 — data browser UI (client, Stig)
implant/browser(key B, fullscreen), sibling to the Atlas per Jeroen's IA ruling: kind picker → generic filterable index (live search-mode typing) → generic detail, parameterized per kind, composed entirely from D-169 components — no new UI primitives.available_in_companionleft at default true: the app appears in the companion shell automatically via the D-254 §3 generic-host seam — zero companion-side wiring (the seam's first real test, passed).browser_adapter.gdis the sole home of literal wire field names — all six detail mappers fold join data (corporation presence, commodity production chains with nested Leontief inputs, JSON-text opaque columns rendered raw rather than dropped).browse_protocol.gdsplit from protocol.gd (max-file-lines).proper_name) bypassesDictionary.getfallbacks → rendered literal"<null>". Fixed with a null-vs-absent helper across all six mappers + 4 regression tests. Exactly the bug class the live-Gauntlet-over-mocks preference exists for.fix — spawn-mode systems.db resolution (server, Oscar; found by Stig live)
Pre-existing: the server opened
data/systems.dbcwd-relative, and spawned servers inherit Godot's cwd, which isclient/(--path clientchdirs) — so CultureResolver/CityContextReader/BodySourceResolver/BrowseReader all silently failed in everymake atlas/make gamespawn. Wave 1 masked it (star map serves fromstar_map_data.json). Now exe-anchored (<exe_dir>/../../data/systems.dbfirst, cwd fallbacks for manual runs),world_rootderived from the resolved absolute path so its ancestor arithmetic holds unconditionally. 5 unit tests on the pure candidate-ordering seam; all three launch styles manually verified —make atlasspawn shows all 5 reader fds open onserver/data/systems.dbvia/proc/PID/fd.Verification
make atlasspawn mechanism (start_with_pipe+--port 0+ LISTENING parse, no pre-existing server): identical counts, zero reader-unavailable warnings in the spawned server's log.Tickets: T-1131, T-1133
browse_reader.rs: BrowseReader on the CityContextReader::open() pattern — six index + six detail reads against systems.db (bodies filterable by containing system; system/corporation/commodity details fold their join partners). browse_proxy.rs: BrowseRequest{browse, kind, query} / BrowseResponse{kind, status, index, detail} wire types + dispatcher; BrowseIndexRow{id, primary, secondary} generic across kinds; BrowseDetail a per-kind enum of field-exhaustive structs. Demux: Inbound::BrowseRequest is the FIFTH map shape — deliberately the last; the doc's four-shape ceiling is re-pinned at five with rationale (six kinds x two forms folded into ONE envelope whose internal enums pick sub-behavior, the AtlasLayerRequest.up_to precedent, instead of twelve top-level shapes) and a hard rule that a sixth shape must migrate to the D-225 tagged-envelope framing. Served for BOTH roles, connection-tagged 1:1 in-order like atlas/starmap/citynames. serve_browse_requests pub so integration tests drive the true end-to-end pipeline. Wire-only per D-254 (T-949 precedent); v1 exclusions (cascade geometry, event logs) respected. 30 unit tests + 7 bridge_tcp integration tests (six-kind round-trip over real TCP, reader-can-browse, no crossed responses between two readers, unknown-id NotFound, empty-table Ready); 2 pre-existing tests updated for the new receive_bridge_inputs parameter. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>New implant app (app_path implant/browser, key B, fullscreen), sibling to the Atlas per Jeroen's IA ruling: kind picker -> generic filterable index (live search-mode typing) -> generic detail, parameterized per kind, composed entirely from D-169 components. available_in_companion left unset (default true) — the app appears in the companion shell automatically via the generic-host seam, zero companion-side wiring. browser_adapter.gd is the sole home of literal wire field names: maps Oscar's BrowseResponse contract ({id, primary, secondary} index rows; BrowseDetail enum-as-single-key-map) to view models for all six kinds, folding join partners (system economy/factions/culture, corporation presence, commodity production chains with nested Leontief inputs). browse_protocol.gd split out of protocol.gd (max-file-lines); sim_bridge gains browse_response_received + request_browse_index/detail. Live-data catch: a present-but-NULL key (unnamed asteroid belt proper_name) bypasses Dictionary.get fallbacks and rendered '<null>' — _display_or() null-vs-absent helper applied across all six detail mappers, 4 regression tests distinct from the absent-key cases. 43 gdUnit adapter cases; full suite 3074 green. Live-verified against a real server + real systems.db: all six kinds Ready with real row counts (301/3240/466/165/36/28), detail drill-down, NotFound on bogus ids. Spawn-mode DB resolution issue found during verification is pre-existing (cwd-relative data/systems.db) — server-side fix follows separately. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>PathBuf::from("data/systems.db") was cwd-relative, and a spawned server inherits GODOT's cwd — which is client/ (Godot's --path client chdirs), so CultureResolver, CityContextReader, BodySourceResolver and BrowseReader all silently failed in every make atlas / make game spawn. Wave 1 masked it: the star map serves from star_map_data.json, so 'renders 301 systems' never touched the DB; T-1133's browser was the first DB-backed consumer to hit it live (Stig, /proc/PID/cwd). resolve_systems_db_path(): exe-anchored <exe_dir>/../../data/systems.db first (cwd-independent — the actual fix), then cwd-relative data/systems.db (cd server && cargo run), then server/data/systems.db (repo-root invocations); first existing candidate wins, info-logged. Pure systems_db_candidates() split out as the testable seam (5 unit tests — incl. pinning Path::parent() of a bare filename = Some(""), not None). world_root now derives from the resolved ABSOLUTE path so its ancestors().nth(3) arithmetic holds unconditionally instead of silently falling back to ".." when canonicalize failed. Verified all three launch styles; make atlas spawn shows 5 reader fds open on server/data/systems.db via /proc/PID/fd. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Review: atlas-companion-browser → main (type: code) — round 2 (fix round)
Tyre (Architecture): APPROVE — including an explicit ruling on the demux ceiling
Ceiling ruling (the architectural decision of this PR): the fifth hand-sniffed wire shape is APPROVED — five-with-a-hard-wall is correct, and
bridge/mod.rsis the right home for the contract. Reusing an existing shape would overloadAtlasLayerRequest.up_towith unrelated semantics and weaken the one-discriminator-per-shape convention; doing the D-225 tagged-envelope migration now would be scope creep re-litigating a stable four-shape contract mid-Phase-4.BrowseRequestis legitimately "one shape, not twelve" (internal enums pick sub-behavior, theup_toprecedent). Union rejection verified directly:ShapeProbecountsbrowse; multi-discriminator frames reject; a well-formed BrowseRequest shares no field names with other shapes' discriminators. The record's contract going forward: ceiling pinned at five; the sixth new inbound shape triggers the tagged-envelope migration — no sixth probe. The amendment's home (bridge doc + D-225 reference, not D-254) is correct — no record drift.Also verified: read-layer per-kind duplication is repo idiom matching CityContextReader (not debt); detail structs field-exhaustive against the schema with graceful drift behavior (Err → BrowseStatus::Error, never panic); determinism discipline sound (PreInput serve, fast-path early-return, zero per-tick I/O with empty buffers); cwd-fix candidate order and
ancestors().nth(3)arithmetic correct now that the input is an existence-verified absolute path;browser_adapter.gda clean single wire-fieldname seam; the D-254 §3 generic-host seam held with zero companion-side wiring on its first real exercise.Hoshe (QA): APPROVE (round 1: REQUEST_CHANGES, 1 finding — fixed and confirmed)
Verified: all twelve SQL queries cross-checked column-by-column against systems-schema.sql, zero string-concatenated SQL — every wire-supplied parameter bound via rusqlite placeholders, including the
?1 IS NULL OR system_id = ?1optional-filter idiom;two_readers_do_not_cross_browse_responsesis a genuine concurrency assertion (in-flight requests for different kinds, wall-clock-bounded polling, ran 5× no flakiness);_display_ornull-vs-absent handling applied exactly where needed with 4 regression tests genuinely distinct from the absent-key cases; cwd-fix unit tests pin the exe-anchored path, both fallbacks, and twoPath::parent()edge cases;available_in_companioncorrectly inherited by omission; key B collides with nothing.Round-1 finding (fixed in
132915a4e): the union-frame rejection mechanism correctly counts the newbrowsediscriminator (verified live via scratch tests), butambiguous_union_frame_is_rejectedwas carried over from PR #176 without a browse-inclusive case — a coverage hole on a security-relevant demux invariant that a future refactor could silently regress. Fix: two browse-inclusive union cases (browse+star_map, browse+city_names) with genuinely well-formed BrowseRequest fields, both asserting rejection. Re-review: CONFIRMED, APPROVE — "a stronger construction than my own scratch verification"; full browse suite re-run, no regressions.132915a4e)Verdict: APPROVED
Runtime verification: full cargo + gdUnit suites via the pre-push gate on every push. Live against real data in BOTH connection modes: all six kinds Ready with real row counts (301/3240/466/165/36/28), detail drill-down, NotFound on bogus ids; spawn-mode re-verified through the exact make atlas mechanism after the exe-anchored systems.db fix (zero reader-unavailable warnings). Merge-path smoke: connection-handshake surface unchanged from wave 1 (browse rides the established demux); the pre-existing spawn-path DB bug this PR fixes was itself found by live verification.
Pull request closed