Skip to content

docs(roadmap): correct the wire contract and record what has shipped - #27

Open
btripp wants to merge 3 commits into
open-flight:mainfrom
btripp:docs/roadmap-refresh
Open

btripp wants to merge 3 commits into
open-flight:mainfrom
btripp:docs/roadmap-refresh

Conversation

@btripp

@btripp btripp commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Updates ROADMAP.md to match the server contract and the code that has shipped. Documentation only; no source or behaviour changes.

Why was this required?

Three things in the roadmap would mislead someone implementing from it.

Phase 1 item 4 specifies set_player / player_changed, and the contract reference lists both. Neither event exists on the server. Profiles replaced players: the server exposes get_profiles, set_active_profile, add_profile, rename_profile and remove_profile, each answered with a full profiles snapshot, and stamps every shot with profile_id / profile_name. Anyone building that row from this document would write a client against an API that was never there.

The graceful-shutdown row said it "prevents yanking power on a live Pi (SD-card corruption risk)". POST /api/shutdown stops the OpenFlight server process — server.py calls os._exit(0) — and leaves the Pi powered. Neither server.py nor start-kiosk.sh has a poweroff path. Describing it as power safety invites the opposite conclusion, which is why the Device tab that shipped in #26 is worded as stopping OpenFlight.

Nothing recorded what had shipped. Phase 0 is complete, and parts of Phases 1 and 2 have landed, but the document still read as though none of it had. Several rows only partly shipped, so they are marked 🟡 rather than ✅: session stats (the tiles are on the Shots tab and the Stats tab is still a placeholder), the unit toggle (#18 ported only the conversion helpers, which nothing uses yet), and profiles (data layer only, no picker).

Automated tests

None. This changes one Markdown file, so there is no behaviour to test. The Policy check's "product source changed without test changes" rule does not apply, as ROADMAP.md is not product source. The build and test checkboxes below are unticked because no code changed and I did not run them for this branch.

Verification was against sources instead:

  • Status for every row was checked against main at 9b5de01 by grepping each event through app/, components/, services/, stores/, utils/ and types.ts — not inferred from PR titles. That is what caught rows claiming more than shipped, including stats, units and a radar_config type that does not exist.
  • Contract claims were checked against src/openflight/server.py and src/openflight/profiles.py in open-flight/openflight.
  • npx prettier --check ROADMAP.md passes.

Manual (human) testing

No simulator or device run, and none is warranted: no executable code changed and the bundle is unaffected. Saying so plainly rather than claiming a run that did not happen.

Checked by hand: set_player and player_changed appear nowhere in the server source; delete_shot, clear_session and session_cleared are emitted nowhere in this app, so the history and stats rows were claiming a destructive half that never shipped; shot_processing and radar_config exist only in types.ts with no consumer.

Server/API contract impact

None — no client behaviour changes.

The document now describes the existing contract accurately: it drops the non-existent set_player / player_changed, adds the five profile emits plus profiles, shot_update, debug_toggled and delete_shot_error, and records the profile behaviours the wire does not carry — a refused mutation returns an unchanged snapshot with no error event, add_profile also sets the active profile, and MAX_PROFILES (12) and MAX_NAME_LENGTH (40) are enforced server-side but never sent.

AI assistance

AI assistance (Claude Code) drafted this revision after inventorying the server's Socket.IO surface and grepping this app for each event. I reviewed every status claim against the code and the server source before committing.

Checklist

  • This PR has one coherent objective and contains no unrelated changes
  • New or changed behavior has automated tests, or I explained why none apply
  • I documented manual human testing with the actual platform/build used
  • I documented any OpenFlight server contract impact
  • I disclosed substantive AI assistance and personally reviewed every change
  • npm ci succeeds
  • npx expo-doctor passes
  • npx tsc --noEmit passes
  • npm test -- --ci --runInBand passes
  • npx expo export --platform all succeeds when application code changed
  • Documentation was updated where required
  • No policy documents are mixed into a product-feature PR
  • No unrelated generated files, formatting, or dependency updates are included

The roadmap has drifted from the code in ways that would mislead someone
implementing from it.

Phase 1 item 4 specified `set_player` / `player_changed`, and the contract
reference listed both. Neither event exists on the server. Profiles replaced
players: the server exposes get_profiles, set_active_profile, add_profile,
rename_profile and remove_profile, answered by a `profiles` snapshot, and
stamps every shot with profile_id / profile_name. Anyone building Phase 1
item 4 from this document would have written a client against an API that
was never there.

The graceful-shutdown row said it "prevents yanking power on a live Pi".
POST /api/shutdown stops the OpenFlight server process — server.py calls
os._exit(0) — and leaves the Pi powered; no poweroff path exists in either
server.py or start-kiosk.sh. Describing it as a power-safety measure invites
exactly the wrong conclusion, so it is now worded as stopping the server,
matching the UI that shipped in open-flight#26.

Also:

- Adds a status column so each item says whether it shipped and in which PR,
  checked against main at 9b5de01 rather than against PR titles. Two rows
  were bundling a feature with an unshipped destructive half — history
  shipped but delete did not, stats shipped but clear did not — so those are
  split into 1/1b and 2/2b.
- Records that `shot_processing` and `radar_config` are typed but consumed by
  nothing, so a declared type is not evidence a feature exists.
- Corrects two references to Expo SDK 54; the app moved to 57 in open-flight#10.
- Adds the profile behaviours a client has to know because the wire does not
  carry them: refusals are indistinguishable from successes, add_profile also
  sets the active profile, and MAX_PROFILES/MAX_NAME_LENGTH are enforced
  server-side but never sent.

Documentation only; no source or behaviour changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QvMGAzMMWRuxRht8aGciAv
Three rows claimed more than main (9b5de01) delivers:

- Session stats: the Stats tab is still a placeholder. The summary tiles
  live on the Shots tab and are computed on the device by
  utils/sessionStats.ts. They are not read from session_state, whose
  payload has no stats field.
- Unit toggle: open-flight#18 ported only the conversion helpers. Nothing imports
  utils/units.ts, CurrentShotView still hardcodes mph/yds, and there is
  no toggle.
- radar_config: types.ts declares no such type, so the rows describing
  it as typed but undisplayed were wrong.

Also records that the default server URL is not yet the AP address, and
lists the get_debug_status / debug_status pair the Device tab uses.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

@johnpacino johnpacino left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few items...

P2 — Stats and unit selection are overstated as shipped. In the Phase 1 table:

  • The Stats tab is still a placeholder. #20 added locally computed summaries inside the Shots session detail; it does not consume session_state.stats.
  • #18 added conversion helpers only. There is no unit toggle, persisted unit preference, or use of those helpers by the screens; displays still use mph and yards.
    Both should be 🟡 and describe precisely what remains.

P2 — The wire-contract inventory is still inaccurate. The roadmap claims radar_config is modeled in types.ts, but the existing source explicitly says it is not modeled. Conversely, the shipped Device feature emits get_debug_status and consumes debug_status, but both are absent from the contract reference. The “consumed” list also mixes shipped handlers with planned, currently unconsumed events; those should be distinguished.

P3 — Overlength profile names do not silently no-op. The server truncates names to 40 characters. Reaching the 12-profile limit is rejected, but exceeding MAX_NAME_LENGTH is handled differently. The roadmap should state those separately.

Follow-up to a41d220, which flipped the stats and units rows but left the
contract reference and the profile-limit note untouched.

The reference listed every event the roadmap mentions in two flat lists, so a
reader could not tell a registered handler from an event nothing listens to
yet. `shot_processing`, `session_cleared` and `delete_shot_error` sat beside
`session_state` and `shot` as though all five were wired. Emits and events are
now split into shipped and planned, each planned name carrying the item number
that will consume it. That also drops `get_radar_config` / `radar_config`,
which appeared as both an emit and a consumed event despite radar-config
editing being a stated non-goal and nothing in the app touching either name,
and adds the `get_debug_status` / `debug_status` pair the Device tab has
emitted and handled since open-flight#26.

The profile-limit note treated MAX_PROFILES and MAX_NAME_LENGTH as one silent
no-op. They differ. ProfileStore.add() returns None at 12 profiles and creates
nothing, whereas clean_profile_name truncates an overlength name to 40
characters and the mutation then succeeds under the shortened name — the user
gets a profile, just not the one they typed. A blank or whitespace-only name is
a third outcome, rejected by both calls. Each is stated separately, with what a
client should do about it.

Two smaller corrections in passing, both from the same review:

- Phase 1 item 5 still declared "persisted" in its emits/consumes column while
  its own note said no preference is persisted. The column now reads
  client-side, and the note names the remaining work.
- Phase 0's covered-types list omitted `debug_status` and `debug_toggled`,
  which types.ts has declared since open-flight#26.

Documentation only; no source or behaviour changes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QvMGAzMMWRuxRht8aGciAv
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants