feat(flights): context-ranked import with previous-flight and Autorouter

[roznet]

What & why

Import was two bordered buttons (Flight Plan, Weather) that filled the route step and stopped. This makes it one control with four methods, and makes an import reach the people step too.

Design doc: designs/flight-import.md (indexed, linked from ios-app.md).

UI

• One primary button showing the context-ranked method — a flight plan on the clipboard, else a previous flight, else weather — with the full list a chevron away.

  Ranked by context rather than by what was used last, which was the original sketch. Two of the four methods are contextual rather than habitual (a clipboard plan is only useful when there is one; a repeat trip is overwhelmingly a previous flight), a sticky label moves under the pilot, and a fresh install has no last-used value to show.

• Unavailable methods are listed greyed with the reason, not hidden, so "Import from Autorouter" is discoverable before the account is linked.

• The flights-list + is a menu: New Flight / Repeat route / Import…, so repeating last week's trip is two taps without opening the form first.

• The people step offers "Same as EGTF > LFRM" — the crew and passengers of the most recent flight in the same aircraft, falling back to the most recent flight, falling back to isUsualCrew. The route step is four fields; the people step was where the typing actually was.

Structure

• FlightDraft is what every method produces, and NewFlightFlow.apply(_:) is the only writer of form state. A fifth method is a case plus a sheet and touches neither the form nor the control.

  Deliberately not FlightExchange: that is the cross-app wire format and is PII-free by design, so it cannot carry a repeated flight's crew. FlightExchange stays on the wire, FlightDraft is the in-app type.

• Flight.nextOccurrence holds the reschedule rule alone, unit tested: keep the time of day read in the origin airport's zone (a pilot thinks "the 09:00 out of EGTF", not "the 08:00Z"), land today if it is still ahead and tomorrow otherwise, and carry the leg's own duration so an overnight leg is not flattened onto the departure day.

• Autorouter reuses flyfun-common's shared client through a mounted create_autorouter_routes_router(). The app parses the returned fplan on-device with RZFlight.ICAOFlightPlanParser rather than making a second round trip, so it shares one parser with the clipboard path.

  Linking stays on the weather web app: the flyfun apps share one database and one Fernet key, so an account linked there is linked here, and no second redirect URI is registered with Autorouter. Forms mounts the routes router without the linking router.

Fixes found along the way

• A clipboard that did not parse hit guard … else { return } and silently did nothing, which read as a dead button. It now says what is wrong.
• The suggestion is gated on the non-prompting UIPasteboard.hasStrings, so the system "Allow Paste?" alert appears only once the pilot commits to the method, not on every tap.
• createReturnFlight / createNextLeg wrote departureDate directly, leaving departureTimeUTC empty and departureInstant nil, so the new flight read as having no time entered at all. All three duplication actions go through the departureDateTime / arrivalDateTime setters now, which dual-write the legacy pair and the instant.

Testing / verification

• Python: 5 new tests in tests/integration/test_autorouter_routes.py (routes for a linked account, 409 rather than an empty list when unlinked, 502 when Autorouter is down, the path the iOS client hard-codes, and that the linking flow is not mounted here). 187 tests pass.

  • tests/unit/test_fillers.py has 2 failures around PDF flattening. They fail identically with these changes stashed, so they are pre-existing and unrelated.

• Swift: flyfun-formsTests/FlightImportTests.swift adds 18 tests over Flight.nextOccurrence, FlightDraft, the ranking, and PeopleSuggestion — including the overnight-leg duration case and the origin-zone case.

  ⚠️ None of the Swift has been compiled or run. There is no Swift toolchain in the environment this was written in. I reviewed it adversarially and fixed three issues that way (a mixed-type foregroundStyle ternary, a start property colliding with a start(_:) method, main-actor isolation on the pasteboard reads), but the first xcodebuild is the real check. Please run the unit target before merging.

Merge order

1. feat(autorouter): share the recent-routes client across apps flyfun-common#10 (the shared Autorouter client)
2. refactor(autorouter): use the shared recent-routes client flyfun-weather#615 (weather onto that client)
3. this PR — it pins flyfun-common>=0.6.6

Known limitation

AirportTimezoneCache reverse-geocodes, so the very first repeat of a flight to an airport the app has never resolved falls back to UTC and the rescheduled time can be an hour or two off what the pilot expects. The cache is on disk and shared with the date/time field, so in practice it is warm. Documented in the design doc's gotchas; happy to block briefly on resolution instead if you would rather trade a spinner for it.

---

Generated by Claude Code

[claude]

Code Review

Reviewed the diff against designs/flight-import.md / designs/ios-app.md. Two correctness issues and one design-doc/code mismatch; no CLAUDE.md exists in this repo so that check was skipped.

1. Re-importing doesn't clear fields the new source doesn't provide (bug)

NewFlightFlow.apply(_:) (Views/NewFlightFlow.swift) only sets crew/passengers/responsiblePerson/nature/contact/observations when the incoming FlightDraft has a non-nil/non-empty value for them — it never clears them:

if !draft.crew.isEmpty { selectedCrew = draft.crew }
if !draft.passengers.isEmpty { selectedPassengers = draft.passengers }
if let responsiblePerson = draft.responsiblePerson { selectedResponsiblePerson = responsiblePerson }
if let value = draft.nature { nature = value }
...

Only the "previous flight" method populates these; clipboard/weather/Autorouter drafts leave them empty/nil. So if a pilot imports via "Previous Flight" (bringing crew, nature, observations) and then changes their mind and imports a different flight via clipboard/weather/Autorouter, the stale crew/passengers/nature/observations from the first import silently survive onto what the pilot believes is a fresh import from an unrelated flight. Given this is a customs/immigration form, wrong crew persisting is a real correctness problem, not just a UI nit. Since apply(_:) is documented as "the single writer of form state," it should fully reset these fields to the new draft's values (including clearing them when absent) rather than merging.

2. Autorouter's Availability is gated on isSignedIn, not on the account actually being linked

FlightImportMethod.availability(in:) (Services/FlightImportMethod.swift):

case .weather, .autorouter:
    return context.isSignedIn
        ? .available
        : .unavailable(reason: String(localized: "Sign in to import"))

Both the design doc and the PR description state the point of showing unavailable methods greyed-with-reason is so "Import from Autorouter" is discoverable before the account is linked. But there's no autorouterLinked signal in FlightImportContext (the design doc's own sketch of the context includes one) — a signed-in pilot who has never linked Autorouter sees the method as fully available, taps it, waits for the picker's spinner, and only then gets the 409 "link your account" error, instead of seeing that reason up front in the list.

3. PeopleSuggestion doesn't reuse the co-traveler machinery the design doc says it does

designs/flight-import.md states the suggestion "reuses the existing co-traveler machinery in PeoplePickerView (Person.coTravelers(minimumFlights:), usualCrewGroup) rather than inventing a second notion of 'who normally flies with me'," and PeopleSuggestion.swift's own doc comment repeats this claim. In practice PeopleSuggestion.suggest is a standalone reimplementation (filter/sort over [Flight], matching by aircraft/recency) that never calls Person.coTravelers or usualCrewGroup. Not a functional bug, but it is exactly the "second notion" the comment says was avoided, and the design doc is now inaccurate — worth reconciling (either call the existing helper or correct the doc/comment).

Everything else — Flight.nextOccurrence, the FlightDraft conversions, the Autorouter service/router split, and the departureDateTime/arrivalDateTime setter fix in FlightEditView — looks correct and matches the design doc.

[roznet]

All three findings addressed in 0e9e196.

1. Re-import merged instead of replacing — agreed, real bug, fixed. Importing a previous flight and then importing an unrelated route left the first flight's crew on the form. On a customs document that is the wrong people, not a UI nit.

I did not blanket-clear, though, because that has its own failure mode: a pilot who picks crew by hand on step 2, goes Back, and re-imports the route to fix a typo would have that crew wiped. The replacement is scoped by ownership — peopleCameFromImport tracks whether the current selection came from an import, so an import replaces what an import wrote and leaves a hand-picked selection alone. The form-level fields (responsiblePerson, nature, contact, observations) have no editor in this flow, so an import is their only source and they are replaced unconditionally. Route fields keep their non-empty guards: an empty departure is a parse defect, not an instruction to clear a good value.

Not unit-testable as it stands — apply(_:) is private view state — so this one rests on review, not on a test.

2. Autorouter gated on isSignedIn rather than linked — agreed, fixed. You are right that the design doc promised something the code did not deliver. FlightImportContext.autorouterLinked is now filled from a new GET /api/autorouter/status (roznet/flyfun-common#10), which answers from stored credentials with no call to Autorouter, so it is cheap enough to ask every time the form opens.

Unknown (nil, check still in flight or failed) counts as available: a pending fetch must never hide a method that works, and the picker's 409 still explains it if the answer turns out to be no. Three tests cover linked / not-linked / unknown.

3. PeopleSuggestion does not reuse coTravelers — agreed the doc was wrong; I corrected the claim rather than the code. coTravelers answers "who usually flies with this person" and needs a person already selected to anchor it, so it widens an existing selection. The chip answers "who was on board last time", which is the only one of the two that can fill an empty people step in one tap. They are different questions and both are wanted. The doc comment and designs/flight-import.md now say that explicitly instead of claiming a reuse that was never there.

Still outstanding from the PR description and unchanged by this round: no Swift has been compiled. There is no toolchain in the environment this was written in, so the 20 tests in FlightImportTests.swift have never run.

---

Generated by Claude Code

[claude]

Code Review

Reviewed the full diff (both commits) against designs/flight-import.md and designs/ios-app.md. No CLAUDE.md exists in this repo, so that criterion doesn't apply.

Findings: none blocking.

The implementation matches the design doc closely: FlightDraft as the single import result, apply(_:) as the sole writer of form state, the ownership-scoped people replacement (peopleCameFromImport), the autorouterLinked == nil → treat-as-available handling, and the Flight.nextOccurrence schedule rule are all consistent with what's documented and covered by the new unit tests (FlightImportTests.swift) and Python integration tests (test_autorouter_routes.py).

Specifically checked and found sound:

• The two follow-up fixes in the second commit (replace-not-merge on a second import, gating Autorouter on the link status rather than just sign-in) are correctly wired into NewFlightFlow.apply(_:) and FlightImportMethod.availability(in:).
• createReturnFlight/createNextLeg/duplicateFlight now go through departureDateTime/arrivalDateTime setters instead of writing departureDate directly — matches the stated bug fix, and the pre-existing same-instant departure/arrival quirk in createReturnFlight is unchanged (not introduced by this PR).
• SwiftUI concurrency: NewFlightFlow/AutorouterRoutePickerView etc. conform to View, so the whole struct is main-actor-isolated by inference — the non-async calls to FlightImportContext.pasteboardHasText and ClipboardFlightPlan.read() (both @MainActor) from synchronous view code are fine.
• The multiple-.sheet coordination in NewFlightFlow (import picker vs. method list vs. people picker) correctly serializes presentation via pendingMethod/onDismiss to avoid SwiftUI dropping a second sheet transition in the same turn.
• Python: the new router mount is read-only and correctly omits the linking router; tests cover the 409/502/mounted-paths/status-without-network cases described in the PR body.

One thing to double check before merging, since the PR description already flags that none of the Swift has been compiled: FlightDraft.init(repeating:) (Models/FlightDraft.swift) constructs a new RZFlight.Route(departure:destination:aircraftType:departureTime:arrivalTime:) — this is the one call in the diff that isn't just passing through an already-constructed Route, so it's worth confirming that initializer signature against the actual RZFlight package once xcodebuild runs.

No other correctness, duplication, or architecture issues found. Approving.