diff --git a/.gitignore b/.gitignore index 5194af0..7d9b518 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,7 @@ htmlcov/ # `research/*` not `research/` so git can still see inside the dir. research/* !research/extract_help_docs.py +!research/capture_matrix_spa.py # Claude Code per-session agent state. Project-shared `.claude/settings.json` # and `.claude/skills/` ARE checked in; per-machine override stays local. diff --git a/docs/memories/matrix_spa_url_state.md b/docs/memories/matrix_spa_url_state.md new file mode 100644 index 0000000..6e43521 --- /dev/null +++ b/docs/memories/matrix_spa_url_state.md @@ -0,0 +1,55 @@ +# Matrix SPA URL state vs the /batch API + +Captured 2026-08-01 by driving the real Matrix UI (patchright + real Chrome, +Advanced controls, one-way JFK→LHR with `Routing=BA+`, `Extension=MAXSTOPS 0`). + +## The trap + +The SPA's URL state and the `/batch` API use **different names for the same +values**. Guessing from the API side gets you a link the app ignores. + +| Concept | `/batch` API (`wire.py`) | SPA URL state (`links.py`) | +|---|---|---| +| Routing language | `routeLanguage` | `routing` | +| Extension codes | `commandLine` | `ext` | +| Return-leg routing | (own slice) | `routingRet` | +| Return-leg extension | (own slice) | `extRet` | +| Arrival-date intent | `isArrivalDate: bool` | `departureDateType: "depart"｜"arrive"` | + +Round trip folds into ONE slice, which is why the inbound leg needs the +separate `*Ret` keys rather than a second slice. + +## Presence is conditional + +With no routing codes set the SPA **omits all four keys**; with any set it +emits all four (blank string for the unused ones). `_spa_routing_fields` +mirrors that, so our links stay byte-identical to the app's own in both cases +— the tracked fixtures in `tests/fixtures/matrix_url/` cover both shapes. + +## Capture recipe + +`research/capture_matrix_spa.py` does this **unattended** — re-run it whenever +the SPA changes: + + uv run --with patchright python research/capture_matrix_spa.py + +Headless is blocked by `waa-pa` bot attestation, so it drives real Chrome via +patchright, but needs no human. It records both surfaces at once: `page.url` +(decode the `search=` base64) and `page.on("request")` filtered to +`alkali`/`batch` (the API body). + +Four form-driving traps, each of which silently leaves Search **disabled**: + +1. Airports are an autocomplete — type, then **click the `mat-option`**. + `fill()` leaves the underlying model empty. +2. The date input has **no placeholder**; select it by + `input.mat-datepicker-input`. +3. The date must be typed with **`press_sequentially`**. `fill()` sets the + visible value but does not fire the events Angular's form model listens + for, so Search stays disabled with a date plainly showing — the most + misleading of the four. +4. `mat-input-*` ids are regenerated per render. Never select on them; + `input[placeholder="Routing"]` / `"Extension"` are stable. + +Order matters too: pick airports **before** switching to One way, or the date +control isn't rendered yet. diff --git a/pyproject.toml b/pyproject.toml index 0f5c6d5..a88221e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -26,6 +26,7 @@ dependencies = [ "flights>=0.9", # fli — drives Google Flights for booking handoff "fast-flights>=2.2", # tfs= URL encoder (Google Flights deep-link) "rookiepy>=0.5", # reads Chrome cookies for `auth pp login --from-chrome` + "diskcache>=5.6", # HTTP response cache: per-entry TTL, LRU, SQLite/WAL ] [project.optional-dependencies] diff --git a/research/capture_matrix_spa.py b/research/capture_matrix_spa.py new file mode 100644 index 0000000..c16798d --- /dev/null +++ b/research/capture_matrix_spa.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python +"""Capture the Matrix SPA's URL state + /batch request body, unattended. + +Why this exists: the SPA names things differently from the /batch API +(`routing`/`ext` vs `routeLanguage`/`commandLine`), so `links.py` cannot be +written from the API side — the field names have to be observed. See +docs/memories/matrix_spa_url_state.md. + +Run: uv run --with patchright python research/capture_matrix_spa.py + +Headless is blocked by Google's waa-pa bot attestation, so this drives real +Chrome via patchright. It still runs without supervision. + +Form-driving notes, each learned the hard way: + * Airports are an autocomplete: type, then CLICK the mat-option. `fill()` + leaves the model empty and Search stays disabled. + * The date input has NO placeholder — select it by `input.mat-datepicker-input`. + * The date must be typed with `press_sequentially`; `fill()` sets the value + but doesn't fire the events Angular's form model listens for, so Search + stays disabled with a date visibly present. + * `mat-input-*` ids are regenerated per render — never select on them. +""" + +from __future__ import annotations + +import base64 +import json +import pathlib +import sys +import urllib.parse + +from patchright.sync_api import Page, sync_playwright + +PROFILE = "/tmp/mx-capture-profile" +URL_OUT = pathlib.Path("/tmp/matrix_spa_state.json") +REQ_OUT = pathlib.Path("/tmp/matrix_batch_body.json") + + +def _decode_state(url: str) -> dict | None: + if "search=" not in url: + return None + q = urllib.parse.unquote(url.split("search=", 1)[1].split("&")[0]) + q += "=" * (-len(q) % 4) + try: + return json.loads(base64.b64decode(q)) + except Exception: + return None + + +def _pick_airport(pg: Page, index: int, code: str) -> None: + box = pg.locator('input[placeholder="Add airport"]').nth(index) + box.click() + box.type(code, delay=120) + pg.wait_for_timeout(1800) + option = pg.locator("mat-option, [role=option]").first + option.wait_for(state="visible", timeout=10_000) + option.click() + pg.wait_for_timeout(700) + + +def capture(origin: str, dest: str, date_mmddyyyy: str, routing: str, extension: str) -> int: + bodies: list[dict] = [] + + with sync_playwright() as p: + ctx = p.chromium.launch_persistent_context( + PROFILE, channel="chrome", headless=False, + viewport={"width": 1500, "height": 1000}, + ) + pg = ctx.pages[0] if ctx.pages else ctx.new_page() + + def on_request(req): + if "alkali" not in req.url and "batch" not in req.url: + return + body = req.post_data + if body and ("routeLanguage" in body or "commandLine" in body): + bodies.append({"url": req.url.split("?")[0], "body": body}) + + pg.on("request", on_request) + pg.goto("https://matrix.itasoftware.com/", wait_until="domcontentloaded") + pg.wait_for_timeout(5000) + + _pick_airport(pg, 0, origin) + _pick_airport(pg, 1, dest) + + pg.locator("text=One way").first.click() + pg.wait_for_timeout(1500) + + date_in = pg.locator("input.mat-datepicker-input").first + date_in.click() + pg.wait_for_timeout(600) + date_in.press_sequentially(date_mmddyyyy, delay=90) + pg.wait_for_timeout(800) + pg.keyboard.press("Tab") + pg.wait_for_timeout(1200) + + pg.locator("text=Advanced controls").first.click() + pg.wait_for_timeout(1500) + pg.locator('input[placeholder="Routing"]').first.fill(routing) + pg.locator('input[placeholder="Extension"]').first.fill(extension) + pg.wait_for_timeout(600) + + search = pg.locator('button:has-text("Search")').first + if search.get_attribute("disabled"): + print("Search still disabled — the form shape changed", file=sys.stderr) + ctx.close() + return 1 + search.click() + + state = None + for _ in range(90): + pg.wait_for_timeout(1000) + found = _decode_state(pg.url) + if found and (found.get("slices") or [{}])[0].get("routing"): + state = found + break + if found: + state = found + if bodies: + break + + if state: + URL_OUT.write_text(json.dumps(state, indent=1)) + print(f"URL state -> {URL_OUT}") + print(json.dumps(state["slices"][0], indent=1)) + if bodies: + REQ_OUT.write_text(json.dumps(bodies, indent=1)) + print(f"batch bodies -> {REQ_OUT} ({len(bodies)})") + ctx.close() + return 0 if state else 1 + + +if __name__ == "__main__": + raise SystemExit( + capture( + origin="JFK", dest="LHR", date_mmddyyyy="09/01/2026", + routing="BA+", extension="MAXSTOPS 0", + ) + ) diff --git a/src/flight_cli/_gflight_ids.py b/src/flight_cli/_gflight_ids.py index c6a5a39..3187cc1 100644 --- a/src/flight_cli/_gflight_ids.py +++ b/src/flight_cli/_gflight_ids.py @@ -434,8 +434,9 @@ def _seed_cookies_once(client: Any) -> None: log.debug("gflight cookie cache past TTL; re-warming") return try: + jar = _cookie_jar(client) for c in saved: - client._client.cookies.set( + jar.set( c["name"], c["value"], domain=c.get("domain", ".google.com"), @@ -445,6 +446,27 @@ def _seed_cookies_once(client: Any) -> None: log.debug("could not seed gflight cookies: %s", e) +def _cookie_jar(client: Any) -> Any: + """The session cookie jar, across fli client shapes. + + fli <=0.8 exposed `Client._client`; 0.9 replaced it with a per-thread + `Client._session()`. Both hand back an object with the same `.set()` / + `.jar` interface. Reaching for the old attribute silently raised + AttributeError into this module's best-effort `except`, which turned NID + seeding AND persistence into no-ops — so every process started cold, and + the comments above put the cold-start empty rate at ~40% versus ~0% warm. + + Raises AttributeError when neither shape is present, so a future upstream + rename fails loudly at the callers' `except` + debug log rather than + degrading silently forever. + """ + session = getattr(client, "_client", None) + if session is None: + # fli 0.9: per-thread session accessor replaced the old `_client` attr. + session = client._session() # pyright: ignore[reportAny] + return session.cookies # pyright: ignore[reportAny] + + def _persist_cookies(client: Any) -> None: """Write the session's allowlisted Google cookies (NID) to disk after a warm call, once per process, so the next invocation starts warm. Best-effort.""" @@ -458,7 +480,7 @@ def _persist_cookies(client: Any) -> None: "domain": str(ck.domain or ".google.com"), "path": str(ck.path or "/"), } - for ck in client._client.cookies.jar # pyright: ignore[reportAny] # fli/curl_cffi untyped + for ck in _cookie_jar(client).jar # pyright: ignore[reportAny] # fli/curl_cffi untyped if str(ck.name) in _PERSIST_COOKIE_NAMES and _GOOGLE_DOMAIN_SUFFIX in str(ck.domain or "") ] diff --git a/src/flight_cli/_http.py b/src/flight_cli/_http.py index 5398d06..1e22186 100644 --- a/src/flight_cli/_http.py +++ b/src/flight_cli/_http.py @@ -11,6 +11,8 @@ from typing import TYPE_CHECKING, Any, cast import anyio +import anyio.to_thread +import diskcache # pyright: ignore[reportMissingTypeStubs] # DIVERGE: no stubs shipped; Profile-B edge import httpx import stamina import structlog @@ -55,13 +57,49 @@ def _is_retryable(exc: Exception) -> bool: return False +# How long a cached search body may be served. Matrix is research-only — the +# booking handoff is the Google Flights link — so a slightly stale fare costs a +# re-search, never a bad purchase. Long enough to dedupe the repeat queries a +# single command makes (multi-cabin merge, calendar fan-out, a `--pick` +# re-render); short enough that nobody acts on a quarter-hour-old fare. +CACHE_TTL_SECS = 15 * 60 + +# Bounds the cache directory. Entries are ~20 KB, so this is generous; without +# it the store only grows, which is what the previous hand-rolled cache did +# (77 days of orphans, no eviction). +CACHE_SIZE_LIMIT_BYTES = 256 * 1024 * 1024 + + +def is_cacheable_body(data: dict[str, Any]) -> bool: + """Whether a decoded response body may be stored. + + Matrix signals failure with HTTP 200 plus `{"error": {...}}` (see + `MatrixApiError`), so status code alone cannot gate this and no HTTP-aware + cache library can either — the judgement needs Matrix's own shape. Caching + those bodies made a transient brownout permanent: a live cache inspected + during review held 19 such entries, including `Internal server error`, each + replayed forever because nothing expired. + """ + return "error" not in data + + class HttpTransport: """Wraps httpx + curl_cffi with rate-limit, retry, and optional disk cache. The cache is content-addressed by (url, sorted-JSON-body) and stores the - decoded response body as JSON. It exists so we can develop the CLI / parser - against captured responses while Matrix is in one of its frequent - empty-calendar brownouts. + decoded response body. It exists so we can develop the CLI / parser against + captured responses while Matrix is in one of its frequent empty-calendar + brownouts, and so one command's repeated queries hit the network once. + + Storage is `diskcache` (SQLite/WAL) rather than hand-rolled files. That + choice was measured, not assumed: `diskcache` is sync-only, but through + `anyio.to_thread` (which reuses its worker pool) a set costs ~0.15 ms + against ~0.12 ms for a raw JSON write — noise beside a 30-45 s Matrix + call, and the upstream's own async caveat about executor overhead applies + to `asyncio.run()` spawning a fresh pool per call, which we don't do. In + exchange we get per-entry TTL, LRU eviction and a size cap from a tested + implementation; this project had already shipped two expiry bugs in + hand-rolled caches (mtime-refresh, restamp-on-read). """ def __init__( @@ -75,6 +113,7 @@ def __init__( cache_dir: pathlib.Path | str | None = None, cache_read: bool = True, cache_write: bool = True, + cache_ttl: float = CACHE_TTL_SECS, ) -> None: self._transport = AsyncCurlTransport( # `impersonate` is a curl_cffi BrowserTypeLiteral string at runtime; @@ -100,8 +139,13 @@ def __init__( ) self._cache_dir = pathlib.Path(cache_dir) self._cache_dir.mkdir(parents=True, exist_ok=True) + self._cache: Any = diskcache.Cache( # pyright: ignore[reportUnknownMemberType] + str(self._cache_dir / "http"), + size_limit=CACHE_SIZE_LIMIT_BYTES, + ) self._cache_read = cache_read self._cache_write = cache_write + self._cache_ttl = cache_ttl async def __aenter__(self) -> HttpTransport: return self @@ -111,33 +155,55 @@ async def __aexit__(self, *_: object) -> None: async def aclose(self) -> None: await self._client.aclose() + self._cache.close() # ────────────────────────── cache helpers ───────────────────────────── - def _cache_key(self, url: str, body: dict[str, Any] | None) -> str: + def _cache_key( + self, + method: str, + url: str, + params: dict[str, Any] | None, + body: dict[str, Any] | None, + ) -> str: + """Hash a canonical envelope of everything that varies the response. + + Method is included so a GET and a POST to one URL can never share an + entry. Params are hashed as structured pairs rather than flattened into + the URL: the old `"&".join(f"{k}={v}")` was unescaped, so a value + CONTAINING a separator forged another request's key — `{"a": "1&b=2"}` + and `{"a": "1", "b": "2"}` produced the identical hash, and whichever + ran first served the other its flight data. + """ + envelope = { + "method": method.upper(), + "url": url, + "params": sorted((str(k), str(v)) for k, v in (params or {}).items()), + "body": body, + } h = hashlib.sha256() - h.update(url.encode()) - if body is not None: - h.update(json.dumps(body, sort_keys=True, separators=(",", ":")).encode()) + h.update(json.dumps(envelope, sort_keys=True, separators=(",", ":")).encode()) return h.hexdigest()[:24] - def _cache_path(self, key: str) -> pathlib.Path: - return self._cache_dir / f"{key}.json" - - def _cache_get(self, key: str) -> dict[str, Any] | None: - p = self._cache_path(key) - if not p.exists(): - return None + async def _cache_get(self, key: str) -> dict[str, Any] | None: + """Fetch a live entry, or None. Expiry is diskcache's, keyed per entry.""" try: - return cast("dict[str, Any]", json.loads(p.read_text())) - except (OSError, json.JSONDecodeError) as e: + hit: Any = await anyio.to_thread.run_sync(self._cache.get, key) # pyright: ignore[reportUnknownArgumentType] + except Exception as e: # noqa: BLE001 — a cache read must never fail a request log.warning("cache_read_failed", key=key, error=str(e)) return None + return cast("dict[str, Any] | None", hit) - def _cache_put(self, key: str, value: dict[str, Any]) -> None: + async def _cache_put(self, key: str, value: dict[str, Any]) -> None: + """Store a response body unless it is an application-level error.""" + if not is_cacheable_body(value): + log.debug("cache_skip_error_body", key=key) + return try: - self._cache_path(key).write_text(json.dumps(value, indent=2)) - except OSError as e: + await anyio.to_thread.run_sync( + lambda: self._cache.set(key, value, expire=self._cache_ttl), # pyright: ignore[reportUnknownMemberType] + ) + except Exception as e: # noqa: BLE001 — a cache write must never fail a request log.warning("cache_write_failed", key=key, error=str(e)) # ───────────────────────────── public API ────────────────────────────── @@ -145,12 +211,9 @@ def _cache_put(self, key: str, value: dict[str, Any]) -> None: async def get_json( self, url: str, *, params: dict[str, Any] | None = None, cache: bool = True ) -> dict[str, Any]: - cache_key = self._cache_key( - url + "?" + "&".join(f"{k}={v}" for k, v in (params or {}).items()), - None, - ) + cache_key = self._cache_key("GET", url, params, None) if cache and self._cache_read: - hit = self._cache_get(cache_key) + hit = await self._cache_get(cache_key) if hit is not None: log.debug("cache_hit", method="GET", url=url) return hit @@ -176,7 +239,7 @@ async def _send() -> httpx.Response: r.raise_for_status() data = r.json() if cache and self._cache_write: - self._cache_put(cache_key, data) + await self._cache_put(cache_key, data) return data async def post_json( @@ -191,12 +254,9 @@ async def post_json( # bodies; we strip that field before hashing so two semantically equal # requests share a cache entry. body_for_hash = {k: v for k, v in body.items() if k != "bgProgramResponse"} - cache_key = self._cache_key( - url + "?" + "&".join(f"{k}={v}" for k, v in (params or {}).items()), - body_for_hash, - ) + cache_key = self._cache_key("POST", url, params, body_for_hash) if cache and self._cache_read: - hit = self._cache_get(cache_key) + hit = await self._cache_get(cache_key) if hit is not None: log.debug("cache_hit", method="POST", url=url) return hit @@ -227,5 +287,5 @@ async def _send() -> httpx.Response: r.raise_for_status() data = r.json() if cache and self._cache_write: - self._cache_put(cache_key, data) + await self._cache_put(cache_key, data) return data diff --git a/src/flight_cli/cli.py b/src/flight_cli/cli.py index c6c600a..41bd5d1 100644 --- a/src/flight_cli/cli.py +++ b/src/flight_cli/cli.py @@ -50,12 +50,13 @@ matrix_itinerary_url, ) from .log import configure as configure_logging +from .models import SearchResult from .pp.auth import load_tokens from .pp.cli import auth_app, run_pp_for_search from .providers.base import LegQuery if TYPE_CHECKING: - from .models import CalendarResult, LegInfo, Location, SearchResult, Slice + from .models import CalendarResult, LegInfo, Location, Slice # Tuple-length sentinels for `--slice` parser (`ORIGIN-DEST:DATE[:r=...:e=...]`). _SLICE_MIN_PARTS = 2 @@ -559,18 +560,37 @@ async def _gather_calendar( just drops its destination from the merge rather than sinking the whole run.""" results: list[CalendarResult | None] = [None] * len(subs) + failures: list[tuple[int, str]] = [] + async def one(i: int, s: CalendarSearch) -> None: try: results[i] = cast("CalendarResult", await c.execute(s, cache=cache)) - except Exception: # noqa: BLE001 — a sub-query failure just drops that destination + except Exception as e: # noqa: BLE001 — a sub-query failure just drops that destination results[i] = None + failures.append((i, str(e) or type(e).__name__)) async with anyio.create_task_group() as tg: for i, s in enumerate(subs): tg.start_soon(one, i, s) + + # A dropped sub-query silently removes its destination from the grid, so + # "cheapest destination" would be computed over an incomplete set and + # presented as the answer. Name what is missing. + for i, msg in sorted(failures): + route = _calendar_route_label(subs[i]) + err.print(f"[yellow]{route}: sub-query failed, omitted from the grid — {msg}[/]") + return [r for r in results if r is not None] +def _calendar_route_label(s: CalendarSearch) -> str: + """`JFK,EWR→LHR` for a sub-query, for failure messages.""" + if not s.legs: + return "?" + leg = s.legs[0] + return f"{','.join(leg.origins)}→{','.join(leg.destinations)}" + + def _run_calendar( search: CalendarSearch, *, @@ -618,7 +638,9 @@ async def go() -> tuple[CalendarResult, int]: return cast("CalendarResult", await c.execute(search, cache=not no_cache)), 0 recovered = await _gather_calendar(c, subs, cache=not no_cache) merged = merge_calendar_results(recovered) - return (merged, n) if not is_empty_calendar(merged) else (merged, 0) + # Count what SUCCEEDED. Returning `n` (the requested fan-out size) + # reported a complete merge even when sub-queries had been dropped. + return (merged, len(recovered)) if not is_empty_calendar(merged) else (merged, 0) try: return anyio.run(go) @@ -729,7 +751,11 @@ async def _go() -> None: _emit_urls(search, matrix_url=matrix_url, google_url=google_url) -def _pinned_solution_index(result: SearchResult | None, pick: int | None) -> int | None: +def _pinned_solution_index( + result: SearchResult | None, + pick: int | None, + rendered: int | None = None, +) -> int | None: """0-based index into `result.solutions` of the itinerary to pin in a deep link. `pick` is the 1-based itinerary number the user saw in the table; None pins the cheapest (row 1). Out-of-range picks warn and fall back to @@ -739,9 +765,13 @@ def _pinned_solution_index(result: SearchResult | None, pick: int | None) -> int return None if pick is None: return 0 - if pick < 1 or pick > len(result.solutions): + # Bound by what the user could actually SEE. Validating against + # `len(result.solutions)` accepted a `--pick` beyond the rendered table and + # then labelled the link "itinerary #N pinned" for a row never displayed. + upper = len(result.solutions) if rendered is None else min(rendered, len(result.solutions)) + if pick < 1 or pick > upper: console.print( - f"[yellow]--pick {pick} is out of range (1-{len(result.solutions)}); " + f"[yellow]--pick {pick} is out of range (1-{upper}); " f"pinning the cheapest itinerary instead.[/]" ) return 0 @@ -802,6 +832,51 @@ def _try_pinned_gflight_url(search: Search, result: SearchResult | None, idx: in return None +def _overlay_awards( + matrix_res: SearchResult, + *, + legs: tuple[Leg, ...], + opts: Any, + sel: Any, + awards_only: bool, +) -> None: + """Render the award overlay for an already-fetched Matrix result.""" + p = opts.pax + run_pp_for_search( + matrix_res, + legs=_build_pp_legs(legs), + num_passengers=_seated_pax(p), + airlines=sel.pp_airlines() if sel is not None else None, + cabins=sel.pp_cabins() if sel is not None else None, + pp_only=awards_only, + json_out=False, + provider_filter=sel.provider_filter if sel is not None else None, + seats_sources=sel.seats_sources() if sel is not None else None, + cash_per_cabin=_cash_per_cabin_single(matrix_res, opts.cabin), + ) + + +def _pin_source_from_merged(rows: list[Any], top_n: int, src: SearchResult) -> SearchResult: + """The itineraries `--pick N` should index: exactly the rows rendered. + + `--pick N` names a row number the user just read off the merged GF+Matrix + table. Pinning from `matrix_res` instead indexes a DIFFERENT sequence — + a Google-only row is cheaper, so it sorts early and shifts every row after + it, and the emitted link then pins an itinerary the user never selected. + Since that link is the handoff to an actual booking, the two orderings + must be the same list. + """ + kept = rows[:top_n] + # `session` / `solution_set` are server-generated and required to build a + # Matrix pinned itinerary URL, so they must survive the re-key. + return SearchResult( # pyright: ignore[reportCallIssue] # DIVERGE: Field(alias=...) confuses basedpyright into requiring aliased names + solutionCount=len(kept), + solutions=[r.itinerary for r in kept], + session=src.session, + solutionSet=src.solution_set, + ) + + def _emit_urls( search: Search, *, @@ -809,8 +884,9 @@ def _emit_urls( google_url: bool, result: SearchResult | None = None, pick: int | None = None, + rendered: int | None = None, ) -> None: - idx = _pinned_solution_index(result, pick) + idx = _pinned_solution_index(result, pick, rendered) # Only claim "#N" when we actually honored the user's pick; an out-of-range # pick falls back to idx 0 and must not mislabel the cheapest as "#N". pinned_label = ( @@ -839,6 +915,8 @@ def _emit_urls( else: console.print("[dim]Google Flights (tfs= structured):[/]") console.print(f" [link]{google_flights_url(search)}[/]") + for note in _gflight_url_caveats(search): + console.print(f" [yellow]note: {note}[/]") except Exception as e: # noqa: BLE001 - third-party undocumented errors; non-fatal fallback console.print(f"[dim]Google Flights link: {e}[/]") @@ -846,6 +924,32 @@ def _emit_urls( # ─────────────────────────── result renderers ────────────────────────────── +def _gflight_url_caveats(search: Search) -> list[str]: + """Ways the emitted Google link is NARROWER than the search it came from. + + `fast_flights`' tfs= encoder takes exactly one origin and one destination + per leg and has no routing-language field, so a multi-airport or routed + search silently degrades: rows flying EWR->LGW under `--routing AA+` sat + beside a link that searched JFK->LHR unconstrained, and nothing said so. + The Matrix link on the same output IS faithful, which made the two + disagree with no explanation. + + We still emit the link — it is a useful starting point, and booking hands + off to Google — but the ways it differs are now stated. + """ + legs: tuple[Leg, ...] = getattr(search, "legs", ()) or () + notes: list[str] = [] + if legs and any(len(lg.origins) > 1 or len(lg.destinations) > 1 for lg in legs): + first = legs[0] + notes.append( + f"multi-airport search narrowed to {first.origins[0]}→{first.destinations[0]} " + "(Google's link format takes one airport pair)" + ) + if any(lg.route_language or lg.extension for lg in legs): + notes.append("routing/extension codes are not expressible in a Google link") + return notes + + def _parse_iso(s: str) -> datetime | None: """Best-effort parse of a slice timestamp ("YYYY-MM-DDTHH:MM[:SS]").""" if not s: @@ -899,7 +1003,29 @@ def _fmt_slice_cell(s: Slice) -> str: return f"{head}\n{tail}" if tail else head -def _render_search(res: SearchResult) -> None: +def _seated_pax(p: Pax) -> int: + """Occupants needing their own seat. + + An infant IN SEAT buys a seat, so it counts; only a LAP infant does not. + Omitting it made the award query ask for fewer seats than the cash query + on the same run, so an award with too little availability rendered as + bookable for the party. + """ + return p.adults + p.children + p.seniors + p.youth + p.infants_in_seat + + +_DEFAULT_RENDER_LIMIT = 10 # matches the `-n/--page-size` default + + +def _render_search(res: SearchResult, limit: int = _DEFAULT_RENDER_LIMIT) -> None: + """Render the itinerary table, showing at most `limit` rows. + + `limit` MUST be the same bound `--pick` is validated against. It was + hardcoded to 10 while `--pick` checked against `len(res.solutions)`, so + `-n 15 --pick 15` printed 10 rows and then emitted a booking link labelled + "itinerary #15 pinned" for a row the user never saw — with no out-of-range + warning, because 15 was in range for the unrendered list. + """ if res.solution_count == 0: console.print("[yellow]No solutions returned.[/]") return @@ -937,7 +1063,7 @@ def _render_search(res: SearchResult) -> None: st.add_column("carriers") st.add_column("outbound") st.add_column("return") - for i, it in enumerate(res.solutions[:10], 1): + for i, it in enumerate(res.solutions[:limit], 1): itn = it.itinerary slcs: list[Slice] = itn.slices if itn else [] it_carriers = ",".join((c.code or "?") for c in (itn.carriers if itn else [])) @@ -1128,13 +1254,13 @@ def _run_matrix_path( sys.stdout.write(json.dumps(res.raw, indent=2)) return if not sel.awards_only: - _render_search(res) + _render_search(res, opts.page_size) if run_pp: p = opts.pax run_pp_for_search( res, legs=_build_pp_legs(legs), - num_passengers=p.adults + p.children + p.seniors + p.youth, + num_passengers=_seated_pax(p), airlines=sel.pp_airlines(), cabins=sel.pp_cabins(), pp_only=sel.awards_only, @@ -1144,7 +1270,20 @@ def _run_matrix_path( cash_per_cabin=_cash_per_cabin_single(res, opts.cabin), ) # `res` was cast to SearchResult at the top of this function; safe to pass through. - _emit_urls(search, matrix_url=matrix_url, google_url=google_url, result=res, pick=pick) + _emit_urls( + search, + matrix_url=matrix_url, + google_url=google_url, + result=res, + pick=pick, + rendered=opts.page_size, + ) + + +# How much deeper to fetch when a Tier-2 routing post-filter will discard rows. +# Bounded rather than unlimited: Google's own result depth is finite and each +# extra page costs a round trip. +_POSTFILTER_OVERFETCH = 5 def _gflight_results(legs: tuple[Leg, ...], opts: SearchOptions, top_n: int) -> list[Any]: @@ -1165,12 +1304,20 @@ def _gflight_results(legs: tuple[Leg, ...], opts: SearchOptions, top_n: int) -> out_constraints = classify(legs[0].route_language, legs[0].extension) if legs else None if out_constraints and out_constraints.predicates: apply_gf_native_filters(fli_filter, out_constraints.predicates) - results: list[Any] = search_with_ids(fli_filter, top_n=top_n) or [] per_slice_preds = [list(classify(lg.route_language, lg.extension).predicates) for lg in legs] + + # Fetch deeper than we need when a Tier-2 post-filter will run, because it + # drops rows AFTER truncation: `-n 1 --routing AA+` fetched exactly one + # itinerary, discarded it for violating the routing, and reported "no + # results" while a qualifying one sat at rank 2. Over-fetching lets the + # filter choose from a real candidate pool; the final slice below still + # honours the user's `top_n`. + fetch_n = top_n * _POSTFILTER_OVERFETCH if any(per_slice_preds) else top_n + results: list[Any] = search_with_ids(fli_filter, top_n=fetch_n) or [] if results and any(per_slice_preds): keep = set(surviving_indices(fli_results_to_search_result(results), per_slice_preds)) results = [r for i, r in enumerate(results) if i in keep] - return results + return results[:top_n] _MERGE_SOURCE_TAG = {"both": "GF+MX", "matrix": "MX", "gf": "GF"} @@ -1274,7 +1421,7 @@ def _run_gflight_path( run_pp_for_search( sr, legs=_build_pp_legs(legs), - num_passengers=p.adults + p.children + p.seniors + p.youth, + num_passengers=_seated_pax(p), airlines=sel.pp_airlines() if sel is not None else None, cabins=sel.pp_cabins() if sel is not None else None, pp_only=awards_only, @@ -1369,28 +1516,25 @@ async def _go() -> None: return matrix_res = cast("SearchResult", matrix_res) - # Repaint: reconciled GF + Matrix, prices attributed. + # Repaint: reconciled GF + Matrix, prices attributed. `--pick` indexes the + # rendered merged rows; with no merged table (awards-only) it falls back to + # Matrix's own ordering, which is then what the user saw. + pin_res = matrix_res if not awards_only: merged = merge_results(fli_results_to_search_result(gf), matrix_res) _render_merged(merged, legs=legs, top_n=top_n) + pin_res = _pin_source_from_merged(merged, top_n, matrix_res) if run_pp: - p = opts.pax - run_pp_for_search( - matrix_res, - legs=_build_pp_legs(legs), - num_passengers=p.adults + p.children + p.seniors + p.youth, - airlines=sel.pp_airlines() if sel is not None else None, - cabins=sel.pp_cabins() if sel is not None else None, - pp_only=awards_only, - json_out=False, - provider_filter=sel.provider_filter if sel is not None else None, - seats_sources=sel.seats_sources() if sel is not None else None, - cash_per_cabin=_cash_per_cabin_single(matrix_res, opts.cabin), - ) + _overlay_awards(matrix_res, legs=legs, opts=opts, sel=sel, awards_only=awards_only) _emit_urls( - matrix_search, matrix_url=matrix_url, google_url=google_url, result=matrix_res, pick=pick + matrix_search, + matrix_url=matrix_url, + google_url=google_url, + result=pin_res, + pick=pick, + rendered=top_n, ) @@ -1682,7 +1826,7 @@ def _run_matrix_path_multi( run_pp_for_search( merged, legs=_build_pp_legs(legs), - num_passengers=p.adults + p.children + p.seniors + p.youth, + num_passengers=_seated_pax(p), airlines=sel.pp_airlines(), cabins=_pp_cabins_for_multi(sel, cabins), pp_only=sel.awards_only, @@ -1749,7 +1893,7 @@ def _run_gflight_path_multi( run_pp_for_search( merged, legs=_build_pp_legs(legs), - num_passengers=p.adults + p.children + p.seniors + p.youth, + num_passengers=_seated_pax(p), airlines=sel.pp_airlines(), cabins=_pp_cabins_for_multi(sel, cabins), pp_only=sel.awards_only, @@ -1860,7 +2004,11 @@ def _render_gflight_table( any_legroom = True t.add_row( label, - f"{fr.currency or 'USD'}{fr.price:.2f}", + # fli types `price` as `NonNegativeFloat | None` ("None when + # not surfaced"). The gflight adapter already tolerates that; + # this renderer formatted it unguarded and raised TypeError, + # so one price-less row took down the whole cash table. + (f"{fr.currency or 'USD'}{fr.price:.2f}" if fr.price is not None else "—"), str(fr.stops), dur, legs_str, @@ -2891,7 +3039,7 @@ def calendar( return if n_split: console.print( - f"[dim]Queried {n_split} destinations separately and merged — Matrix " + f"[dim]Queried {n_split} origin/destination groups separately and merged — Matrix " f"under-reports the combined multi-airport calendar grid.[/]" ) _render_calendar(res, dmin=dmin, dmax=dmax, origin=origins, destination=dests, sd=sd, ed=ed) diff --git a/src/flight_cli/fli_bridge.py b/src/flight_cli/fli_bridge.py index 2d726dd..20cc1aa 100644 --- a/src/flight_cli/fli_bridge.py +++ b/src/flight_cli/fli_bridge.py @@ -111,9 +111,16 @@ def _seg(origin: str, dest: str, dt: str) -> FlightSegment: p = s.options.pax return FlightSearchFilters( + # fli's PassengerInfo takes all four types and permits adults=0, so + # pass the party through as asked. The old `or 1` SYNTHESIZED an adult + # for a child-only search — pricing a 2-passenger trip nobody + # requested — and infants were dropped entirely, so an infant-in-seat + # search silently priced one fewer seat than the Matrix side. passenger_info=PassengerInfo( - adults=(p.adults + p.seniors + p.youth) or 1, + adults=p.adults + p.seniors + p.youth, children=p.children, + infants_in_seat=p.infants_in_seat, + infants_on_lap=p.infants_in_lap, ), flight_segments=segs, stops=stops, diff --git a/src/flight_cli/links.py b/src/flight_cli/links.py index e2ed5c4..91dbff2 100644 --- a/src/flight_cli/links.py +++ b/src/flight_cli/links.py @@ -90,14 +90,45 @@ def _spa_specific_leg(leg: Leg, *, return_leg: Leg | None = None) -> dict[str, A "dates": { "searchDateType": "specific", "departureDate": leg.date.isoformat() if leg.date else "", - "departureDateType": "depart", + # "depart" | "arrive" — the SPA's encoding of arrival-date intent, + # the URL-state counterpart of the API's `isArrivalDate` bool. + "departureDateType": "arrive" if leg.is_arrival_date else "depart", "departureDateModifier": str(leg.date_minus), "departureDatePreferredTimes": [t.value for t in leg.time_ranges], "returnDate": return_date, - "returnDateType": "depart", + "returnDateType": "arrive" if (return_leg and return_leg.is_arrival_date) else "depart", "returnDateModifier": return_modifier, "returnDatePreferredTimes": return_times, }, + **_spa_routing_fields(leg, return_leg), + } + + +def _spa_routing_fields(leg: Leg, return_leg: Leg | None) -> dict[str, str]: + """Routing-language / extension-code keys for a SPA URL-state slice. + + The SPA names these `routing` / `ext` — NOT the `routeLanguage` / + `commandLine` the /batch API uses for the same values. Both shapes were + captured from the real UI: with codes set the slice carries all four keys + (`routingRet` / `extRet` hold the inbound leg's own codes); with none set + it omits them entirely, which is what the tracked fixtures show. We mirror + that, so a link is byte-identical to what the app itself would produce. + + Dropping these meant a link built from `--routing BA+ --ext "MAXSTOPS 0"` + opened an UNCONSTRAINED search — offering the user itineraries the CLI had + deliberately excluded. + """ + routing = leg.route_language or "" + ext = leg.extension or "" + routing_ret = (return_leg.route_language or "") if return_leg else "" + ext_ret = (return_leg.extension or "") if return_leg else "" + if not any((routing, ext, routing_ret, ext_ret)): + return {} + return { + "routing": routing, + "ext": ext, + "routingRet": routing_ret, + "extRet": ext_ret, } @@ -110,11 +141,28 @@ def _spa_specific_slices(legs: tuple[Leg, ...]) -> tuple[str, list[dict[str, Any n = len(legs) if n == 1: return "one-way", [_spa_specific_leg(legs[0])] - if n == _ROUND_TRIP_LEGS: + if n == _ROUND_TRIP_LEGS and _is_inverse_pair(legs[0], legs[1]): return "round-trip", [_spa_specific_leg(legs[0], return_leg=legs[1])] return "multi-city", [_spa_specific_leg(leg) for leg in legs] +def _is_inverse_pair(out: Leg, ret: Leg) -> bool: + """Whether two legs form a true round trip — the return departs where the + outbound landed AND lands where it started. + + Round-trip's SPA encoding folds both legs into ONE slice carrying two + dates, which structurally cannot express a second route. Treating any + 2-leg search as a round trip therefore DELETED the second leg: SFO->JFK + plus LAX->HNL encoded as SFO->JFK with a return date, and LAX/HNL simply + vanished from the emitted link. Multi-city keeps a slice per leg, so + anything that isn't a genuine inverse belongs there. + + Multi-airport legs count as inverse only when the sets match exactly; a + partial overlap is an itinerary we cannot faithfully fold. + """ + return set(out.destinations) == set(ret.origins) and set(out.origins) == set(ret.destinations) + + def _spa_calendar_leg( out: Leg, ret: Leg | None, start: date, end: date, duration_min: int, duration_max: int ) -> dict[str, Any]: @@ -276,6 +324,15 @@ def matrix_deep_link(s: Search) -> str: # tfs= trip-type enum (field 19). _GF_TRIP_ROUND_TRIP = 1 _GF_TRIP_ONE_WAY = 2 +_GF_TRIP_MULTI_CITY = 3 + +# tfs= field 8 is a repeated varint, one entry per occupant, carrying the +# passenger TYPE. Values are Google's own `Passenger` enum, read out of +# fast_flights' generated protobuf (flights_pb2.Passenger) rather than guessed. +_GF_PAX_ADULT = 1 +_GF_PAX_CHILD = 2 +_GF_PAX_INFANT_IN_SEAT = 3 +_GF_PAX_INFANT_ON_LAP = 4 # ───────────────── Google Flights tfs= protobuf (RE'd) ────────────────────── @@ -399,11 +456,18 @@ def _encode_gflight_pinned_tfs( s.message(14, dest_w) w.message(3, s) - # Field 8: one repeated varint=1 per adult (and similar for other pax types - # observed in the captured payload — 3 adults → three "8: 1" entries). - # We replicate the observed pattern: emit one `1` per total occupant. - for _ in range(adults + children + infants_in_seat + infants_on_lap): - w.varint(8, 1) + # Field 8: one repeated varint per occupant, carrying that occupant's TYPE. + # Emitting a bare `1` for everyone encoded children and infants as ADULTS, + # so a pinned link for 1 adult + 1 child priced and searched as 2 adults — + # a different, more expensive itinerary than the row the user picked. + for _type, _count in ( + (_GF_PAX_ADULT, adults), + (_GF_PAX_CHILD, children), + (_GF_PAX_INFANT_IN_SEAT, infants_in_seat), + (_GF_PAX_INFANT_ON_LAP, infants_on_lap), + ): + for _ in range(_count): + w.varint(8, _type) w.varint(9, cabin) w.varint(14, 1) @@ -413,8 +477,16 @@ def _encode_gflight_pinned_tfs( marker.varint(1, (1 << 64) - 1) w.message(16, marker) - # Field 19: trip type. TFS enum: 1 = round-trip, 2 = one-way. - trip_type = _GF_TRIP_ROUND_TRIP if len(slices) >= _ROUND_TRIP_LEGS else _GF_TRIP_ONE_WAY + # Field 19: trip type. TFS enum: 1 = round-trip, 2 = one-way, 3 = multi-city. + # `>= 2` meant round-trip, so a three-leg itinerary was labelled a round + # trip; Google then read only the first two slices and the third leg was + # silently dropped from a link we still described as "pinned". + if len(slices) == 1: + trip_type = _GF_TRIP_ONE_WAY + elif len(slices) == _ROUND_TRIP_LEGS: + trip_type = _GF_TRIP_ROUND_TRIP + else: + trip_type = _GF_TRIP_MULTI_CITY w.varint(19, trip_type) return bytes(w.buf) @@ -531,9 +603,6 @@ def extract_pin_segments_from_slice(s: Slice) -> list[dict[str, str]] | None: dep_date = s.departure[:10] arrival = s.arrival or s.departure arr_date = arrival[:10] - dep_date = s.departure[:10] - arrival = s.arrival or s.departure - arr_date = arrival[:10] out: list[dict[str, str]] = [] for i, fl in enumerate(s.flights): m = _FLIGHT_NUMBER_RE.match(fl) @@ -545,7 +614,16 @@ def extract_pin_segments_from_slice(s: Slice) -> list[dict[str, str]] | None: if has_exact_dates: seg_date = s.segment_dates[i] else: - seg_date = arr_date if (i == n - 1 and arr_date != dep_date) else dep_date + # A segment is dated by when it DEPARTS. The last segment of a + # multi-segment slice departs on the arrival date only when the + # slice spans midnight — and a NONSTOP is never that case, even + # though it satisfies `i == n - 1`: it departs on the departure + # date by definition. Treating an overnight nonstop as arrival- + # dated pinned BA178 JFK->LHR (dep 2026-12-31, arr 2027-01-01) to + # 2027-01-01, sending the user to a search for the wrong day. + spans_midnight = arr_date != dep_date + is_last_of_many = n > 1 and i == n - 1 + seg_date = arr_date if (is_last_of_many and spans_midnight) else dep_date out.append( { "origin": seg_origin, @@ -629,6 +707,11 @@ def google_flights_url(s: Search, *, currency: str = "USD", language: str = "en" infants_in_seat=p.infants_in_seat, infants_on_lap=p.infants_in_lap, ), + # The stop limit is a TFSData-level field, not per-FlightData. Omitting + # it made a `--stops 0` link byte-identical to an unconstrained one, so + # a nonstop-only result table handed the user a page that also offered + # connections. + max_stops=s.options.max_extra_stops, ) b64 = td.as_b64().decode() return ( diff --git a/src/flight_cli/pp/auth.py b/src/flight_cli/pp/auth.py index 5b375b8..6b3a456 100644 --- a/src/flight_cli/pp/auth.py +++ b/src/flight_cli/pp/auth.py @@ -116,9 +116,20 @@ def load_tokens() -> Tokens | None: def save_tokens(t: Tokens) -> None: + """Persist tokens 0600 — the file holds a bearer token to a paid account. + + Created 0600 rather than written and then chmod'd: `write_text` makes the + file at the process umask (0644 by default), so the previous + write-then-tighten left the token world-readable for the window between + those two calls. Opening with the mode up front closes it, and O_TRUNC + keeps the rewrite-in-place behaviour `write_text` had. + """ CONFIG_DIR.mkdir(parents=True, exist_ok=True) - TOKENS_PATH.write_text(json.dumps(t.to_json(), indent=2)) - # 0600 — contains a bearer token to a paid user account. + payload = json.dumps(t.to_json(), indent=2) + fd = os.open(TOKENS_PATH, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w") as fh: + _ = fh.write(payload) + # An existing file keeps its old mode through O_CREAT, so still enforce it. TOKENS_PATH.chmod(0o600) diff --git a/src/flight_cli/pp/cli.py b/src/flight_cli/pp/cli.py index 135c0a4..7c587f3 100644 --- a/src/flight_cli/pp/cli.py +++ b/src/flight_cli/pp/cli.py @@ -387,6 +387,7 @@ async def _go() -> list[list[AwardFlight]]: cabin_list, slice_index=leg.slice_index, cash_per_cabin=cash_per_cabin, + num_passengers=num_passengers, ) @@ -454,21 +455,65 @@ def _fmt_iso_compact(s: str) -> str: def _best_award_for_cabin( award_flights: list[AwardFlight], want_cabin: str -) -> tuple[int, float, str, list[str]] | None: - """Cheapest (lowest miles) offer across providers for one cabin, or None.""" - best: tuple[int, float, str, list[str]] | None = None +) -> tuple[int, float, str, list[str], str, int | None] | None: + """Cheapest offer across providers for one cabin, or None. + + Ranked on (miles, tax) — miles first, since that is the scarce currency, + but ties broken on cash out-of-pocket. Comparing miles alone let a + 30k + $500 offer beat an identical 30k + $6 one purely on provider order. + + Basic-economy fares are excluded here rather than silently rendered as + unrestricted Economy: they carry different seat, bag and change rights, so + presenting one under the plain "Economy" heading overstates what the + traveller gets. `_basic_economy_award_for_cabin` surfaces them explicitly. + """ + best: tuple[int, float, str, list[str], str, int | None] | None = None for af in award_flights: for ca in af.cabins: - if ca.cabin != want_cabin: + if ca.cabin != want_cabin or ca.is_basic_economy: continue - key = (ca.miles, ca.tax_usd, af.program, af.funding_banks) - if best is None or key[0] < best[0]: + key = ( + ca.miles, + ca.tax_usd, + af.program, + af.funding_banks, + ca.tax_currency, + ca.remaining_seats, + ) + if best is None or (key[0], key[1]) < (best[0], best[1]): + best = key + return best + + +def _basic_economy_award_for_cabin( + award_flights: list[AwardFlight], want_cabin: str +) -> tuple[int, float, str, list[str], str, int | None] | None: + """Cheapest BASIC-economy offer for one cabin — the fares + `_best_award_for_cabin` deliberately skips. Rendered with an explicit + label so the restriction is visible rather than implied.""" + best: tuple[int, float, str, list[str], str, int | None] | None = None + for af in award_flights: + for ca in af.cabins: + if ca.cabin != want_cabin or not ca.is_basic_economy: + continue + key = ( + ca.miles, + ca.tax_usd, + af.program, + af.funding_banks, + ca.tax_currency, + ca.remaining_seats, + ) + if best is None or (key[0], key[1]) < (best[0], best[1]): best = key return best def _fmt_award_cell( - award_flights: list[AwardFlight], want_cabin: str, cash_usd: float | None = None + award_flights: list[AwardFlight], + want_cabin: str, + cash_usd: float | None = None, + pax: int = 1, ) -> str: """Render the best (lowest miles) offer across providers for one cabin. @@ -479,23 +524,67 @@ def _fmt_award_cell( columns per cabin) overflows narrow terminals in multi-cabin renders. """ best = _best_award_for_cabin(award_flights, want_cabin) + label = "" + if best is None: + # Nothing unrestricted — show the basic-economy fare rather than an + # empty cell, but say so. + best = _basic_economy_award_for_cabin(award_flights, want_cabin) + label = " [dim](basic)[/]" if best is None: return "—" - miles, tax, program, _banks = best - head = f"{_fmt_miles(miles)} {program} + ${tax:.0f}" + miles, tax, program, _banks, tax_ccy, seats = best + # Print the tax in the currency it is actually denominated in. Formatting a + # EUR amount as "$" both misstates it and invites the reader to add it to a + # USD fare. + tax_str = f"${tax:.0f}" if tax_ccy in ("", "USD") else f"{tax:.0f} {tax_ccy}" + # An award with fewer seats than the party cannot be booked for it. The + # provider reports this; we were discarding it, so a 1-seat fare rendered + # as available for a party of four. `None` means "not reported" (PointsPath + # never does), so only an affirmative shortfall is flagged. + short = ( + f" [yellow]({seats} seat{'s' if seats != 1 else ''})[/]" + if (seats is not None and pax > 0 and seats < pax) + else "" + ) + head = f"{_fmt_miles(miles)} {program} + {tax_str}{label}{short}" if cash_usd is None: return head + # ¢/mi nets the tax off a USD cash fare, so a non-USD tax would silently + # subtract the wrong magnitude. Suppress rather than convert: we have no + # rate source, and a wrong valuation is worse than a missing one. + if tax_ccy not in ("", "USD"): + return head cpm = _cents_per_mile(cash_usd, miles, tax) if cpm is None: return head return f"{head}\n[dim]{cpm:.1f}¢/mi[/]" -def _fmt_funding(award_flights: list[AwardFlight]) -> str: +def _fmt_funding(award_flights: list[AwardFlight], cabins: tuple[str, ...] = ()) -> str: + """Transfer partners that fund the award(s) actually DISPLAYED. + + Unioning banks across every attached award claimed that programs funding + hidden, more-expensive offers also funded the winning one: a 30k Amex + winner beside a hidden 40k Chase offer rendered "Amex, Chase", implying + Chase points could buy the 30k fare. Restricted to the offers the row + shows; `cabins` empty keeps the old union for callers with no cabin + context. + """ + winners: list[tuple[int, float, str, list[str], str, int | None]] = [] + for cab in cabins: + for pick in ( + _best_award_for_cabin(award_flights, cab), + _basic_economy_award_for_cabin(award_flights, cab), + ): + if pick is not None: + winners.append(pick) + sources: list[list[str]] = ( + [w[3] for w in winners] if cabins else [af.funding_banks for af in award_flights] + ) banks: list[str] = [] seen: set[str] = set() - for af in award_flights: - for b in af.funding_banks: + for group in sources: + for b in group: if b not in seen: seen.add(b) banks.append(b) @@ -545,6 +634,7 @@ def _render_matches( *, slice_index: int = 0, cash_per_cabin: Mapping[int, Mapping[str, float]] | None = None, + num_passengers: int = 1, ) -> None: matches = _dedupe_per_leg(matches, slice_index=slice_index) if not matches: @@ -589,9 +679,9 @@ def _render_matches( # — otherwise the value would mix cabins (e.g. business miles vs # economy cash) and mislead. Cabins without per-cabin cash render # the award without a ¢/mi line. - cells.append(_fmt_award_cell(m.awards, cab, per_cabin_cash.get(cab))) + cells.append(_fmt_award_cell(m.awards, cab, per_cabin_cash.get(cab), num_passengers)) if show_funding: - cells.append(_fmt_funding(m.awards)) + cells.append(_fmt_funding(m.awards, tuple(cabin_list))) t.add_row(*cells) console.print(t) @@ -680,11 +770,19 @@ def _serialize_award(af: AwardFlight) -> dict[str, Any]: } -def _serialize_matches(matches: list[MatchedFare]) -> str: +def _serialize_matches(matches: list[MatchedFare], slice_index: int = 0) -> str: + """`slice_index` selects the leg to describe — it MUST match the leg whose + awards are being serialized. Hardcoding slice 0 made the `--json` return + leg report the OUTBOUND flight number, route and departure beside the + return leg's awards, while the wrapper labelled it "return".""" out: list[dict[str, Any]] = [] for m in matches: itn = m.itinerary.itinerary - s = itn.slices[0] if itn and itn.slices else None + s = ( + itn.slices[slice_index] + if itn and itn.slices and slice_index < len(itn.slices) + else None + ) out.append( { "flight": (s.flights[0] if s and s.flights else None), @@ -706,7 +804,7 @@ def _serialize_matches_per_leg( { "leg": leg.label, "slice_index": leg.slice_index, - "matches": json.loads(_serialize_matches(matches)), + "matches": json.loads(_serialize_matches(matches, leg.slice_index)), } for leg, matches in zip(legs, matches_per_leg, strict=True) ], diff --git a/src/flight_cli/pp/client.py b/src/flight_cli/pp/client.py index 6133337..184c915 100644 --- a/src/flight_cli/pp/client.py +++ b/src/flight_cli/pp/client.py @@ -16,7 +16,7 @@ from dataclasses import dataclass from http import HTTPStatus from pathlib import Path -from typing import TYPE_CHECKING, Any +from typing import TYPE_CHECKING, Any, cast import anyio import httpx @@ -37,6 +37,23 @@ API_BASE = "https://api.pointspath.com" + +class PPApiError(Exception): + """A PointsPath endpoint returned an unusable response. + + Exists so `httpx.HTTPStatusError` never reaches a caller (AGENTS.md + Principle 1 — boundaries fail loudly, in domain terms). Per-airline + failures are non-fatal and handled inline; this is for the catalog + endpoints, where a failure means the award overlay cannot be built. + """ + + def __init__(self, message: str, *, endpoint: str, status: int | None = None) -> None: + super().__init__(message) + self.message = message + self.endpoint = endpoint + self.status = status + + PRICING_CACHE = Path.home() / ".cache" / "flight-cli" / "pp_pricing.json" PRICING_TTL_SECS = 24 * 3600 @@ -44,6 +61,23 @@ EXT_CONFIG_TTL_SECS = 7 * 24 * 3600 EXT_CONFIG_VERSION = "1.10.4" +# Airlines the server has told us it does not support, learned at runtime. +# +# PP's /api/pricing-info advertises a superset of what /api/airline-search will +# actually serve: ~10 of its entries (ANA, BritishAirways, CathayPacific, …) +# have no `enable` feature flag, so `enabled_airlines` treats them as +# always-on and every search fans out to them and collects a 400 "unsupported +# airline". That is ~10 wasted round-trips and ~20 warning lines per run. +# +# A hardcoded exclusion list would be wrong: the two conditions look identical +# from the flags alone. AirFrance also has no exact-name flag (only +# `enableAirFranceV2`) and DOES serve results, so "no flag" cannot be the +# predicate. Instead we record the server's own verdict — a 400 naming the +# airline as unsupported — and skip that airline until the TTL expires. The +# list self-heals when PP adds support or the user's tier changes. +UNSUPPORTED_CACHE = Path.home() / ".cache" / "flight-cli" / "pp_unsupported_airlines.json" +UNSUPPORTED_TTL_SECS = 7 * 24 * 3600 + # Airlines observed firing in a single GFlights international search. Used as # the default fan-out set when --pp-airlines isn't provided. Real catalog # probably differs by Pro tier — the extension's /api/extension-config feature @@ -219,12 +253,20 @@ async def airline_search(self, spec: SearchSpec, airline: str) -> AirlineSearchR if r.status_code == HTTPStatus.NO_CONTENT or not r.content: return AirlineSearchResponse() if r.status_code >= HTTPStatus.BAD_REQUEST: - log.warning( - "pp_airline_search_failed", - airline=airline, - status=r.status_code, - body=r.text[:200], - ) + if is_unsupported_airline_response(r.status_code, r.text): + # A permanent "we don't serve this airline", not a transient + # failure — remember it so later runs skip the call entirely. + # Logged at debug: it's an expected steady state, not a problem + # the user can act on. + remember_unsupported_airline(airline) + log.debug("pp_airline_unsupported", airline=airline) + else: + log.warning( + "pp_airline_search_failed", + airline=airline, + status=r.status_code, + body=r.text[:200], + ) return AirlineSearchResponse() return AirlineSearchResponse.model_validate(r.json()) @@ -233,8 +275,16 @@ async def airline_search_many( spec: SearchSpec, airlines: tuple[str, ...], ) -> dict[str, AirlineSearchResponse]: - """Fan out one request per airline; concurrency-bounded by the semaphore.""" + """Fan out one request per airline; concurrency-bounded by the semaphore. + + Airlines the server has previously rejected as unsupported are skipped + without a request — see `UNSUPPORTED_CACHE`. + """ out: dict[str, AirlineSearchResponse] = {} + skip = load_unsupported_airlines() + to_call = tuple(a for a in airlines if a not in skip) + if skipped := tuple(a for a in airlines if a in skip): + log.debug("pp_airline_search_skipped_unsupported", airlines=skipped) async def runner(airline: str) -> None: try: @@ -243,7 +293,7 @@ async def runner(airline: str) -> None: log.warning("pp_airline_search_exception", airline=airline, error=str(e)) async with anyio.create_task_group() as tg: - for a in airlines: + for a in to_call: tg.start_soon(runner, a) return out @@ -253,7 +303,7 @@ async def pricing_info(self, *, force_refresh: bool = False) -> PricingInfoRespo if age < PRICING_TTL_SECS: return PricingInfoResponse.model_validate(json.loads(PRICING_CACHE.read_text())) r = await self._request("GET", "/api/pricing-info") - r.raise_for_status() + _raise_for_status(r, "/api/pricing-info") PRICING_CACHE.parent.mkdir(parents=True, exist_ok=True) PRICING_CACHE.write_text(r.text) return PricingInfoResponse.model_validate(r.json()) @@ -274,7 +324,7 @@ async def extension_config(self, *, force_refresh: bool = False) -> dict[str, An "/api/extension-config", params={"v": EXT_CONFIG_VERSION}, ) - r.raise_for_status() + _raise_for_status(r, "/api/extension-config") EXT_CONFIG_CACHE.parent.mkdir(parents=True, exist_ok=True) EXT_CONFIG_CACHE.write_text(r.text) return r.json() @@ -287,6 +337,110 @@ async def extension_config(self, *, force_refresh: bool = False) -> dict[str, An _AIRLINE_FLAG_RE = re.compile(r"^enable(?P[A-Z][A-Za-z]+?)(?:V\d+)?$") +def _raise_for_status(r: httpx.Response, endpoint: str) -> None: + """Translate a failed response into `PPApiError` at the boundary, so + `httpx.HTTPStatusError` never escapes this module.""" + try: + _ = r.raise_for_status() + except httpx.HTTPStatusError as e: + raise PPApiError( + f"{endpoint} returned HTTP {r.status_code}", + endpoint=endpoint, + status=r.status_code, + ) from e + + +def is_unsupported_airline_response(status: int, body: str) -> bool: + """True when a 4xx says the airline itself isn't served, as opposed to a + transient/auth/route-specific failure. + + Deliberately narrow: it must be a 400 AND the body must name the airline + as unsupported. Broadening this to "any 4xx" would let a rate-limit or an + expired token permanently blacklist a working airline. + """ + if status != HTTPStatus.BAD_REQUEST: + return False + return "unsupported airline" in body.lower() + + +def _load_unsupported_raw() -> dict[str, float]: + """`{airline: epoch_seconds_learned}`, unfiltered. {} on any read problem. + + Tolerates the legacy flat-list format written by the first version of this + cache, dating those entries from the FILE's mtime — the only real timestamp + they have. Stamping them `now` instead would restamp on every read, so a + legacy file could never age out and the entries would be immortal. + """ + try: + raw: Any = json.loads(UNSUPPORTED_CACHE.read_text()) + except (OSError, ValueError): + return {} + if isinstance(raw, list): # legacy: ["ANA", "Finnair", ...] + try: + learned_at = UNSUPPORTED_CACHE.stat().st_mtime + except OSError: + return {} + return {x: learned_at for x in cast("list[Any]", raw) if isinstance(x, str)} + if not isinstance(raw, dict): + return {} + out: dict[str, float] = {} + for k, v in cast("dict[Any, Any]", raw).items(): + if isinstance(k, str) and isinstance(v, (int, float)) and not isinstance(v, bool): + out[k] = float(v) + return out + + +def load_unsupported_airlines() -> frozenset[str]: + """Airlines the server rejected as unsupported, whose note is still fresh. + + Each entry carries its OWN learned-at timestamp. Keying the TTL off the + file's mtime instead would mean any new rejection refreshed every existing + entry — and since a run that learns one airline rewrites the file, entries + would never expire in steady state, defeating the self-healing the TTL is + there to provide. + + Any read problem (missing, corrupt, wrong shape) yields the empty set — + the cost of a stale-empty result is one wasted round-trip per airline, + versus wrongly suppressing a working airline's awards. + """ + cutoff = time.time() - UNSUPPORTED_TTL_SECS + return frozenset(a for a, learned_at in _load_unsupported_raw().items() if learned_at > cutoff) + + +def remember_unsupported_airline(airline: str) -> None: + """Stamp one airline as unsupported as of now. Best-effort: a failure here + only costs the next run a redundant request. + + Existing entries keep their original timestamps so each expires on its own + schedule. + + NOTE: read-modify-write with no lock. Safe within one process — there is no + `await` between the read and the write, so anyio's cooperative scheduling + serializes concurrent callers in `airline_search_many`. Keep it that way: + introducing an async file API here would open a real lost-update race. + Across two concurrent CLI invocations a lost update is possible, and costs + exactly one redundant request next run. + """ + current = _load_unsupported_raw() + now = time.time() + # Re-stamp an EXPIRED entry. Returning early on mere presence meant a + # lapsed airline was re-queried, rejected, and then still looked stale on + # the next run — so it was re-queried forever, exactly what this cache + # exists to avoid. + if current.get(airline, 0.0) > now - UNSUPPORTED_TTL_SECS: + return + current[airline] = now + try: + UNSUPPORTED_CACHE.parent.mkdir(parents=True, exist_ok=True) + # Write-then-rename: a crash mid-write leaves the old file intact + # rather than a truncated one that reads as an empty cache. + tmp = UNSUPPORTED_CACHE.with_suffix(".json.tmp") + tmp.write_text(json.dumps(dict(sorted(current.items())), indent=2)) + tmp.replace(UNSUPPORTED_CACHE) + except OSError as e: + log.debug("pp_unsupported_cache_write_failed", error=str(e)) + + def enabled_airlines( pricing: PricingInfoResponse, ext_config: dict[str, Any], diff --git a/src/flight_cli/pp/gflight_adapter.py b/src/flight_cli/pp/gflight_adapter.py index 2c52baa..e383173 100644 --- a/src/flight_cli/pp/gflight_adapter.py +++ b/src/flight_cli/pp/gflight_adapter.py @@ -97,7 +97,19 @@ def _slice_from_flight_result( def _price_string(fr: Any) -> str: - """Match Matrix's price format ('USD877.00') so match._parse_cash works.""" + """Match Matrix's price format ('USD877.00') so match._parse_cash works. + + fli types `FlightResult.price` as `NonNegativeFloat | None` — "None when + not surfaced", which Google does for some premium round-trip rows that + carry an empty price head. Formatting that unconditionally raised + `TypeError: unsupported format string passed to NoneType.__format__` and + took down the whole search, discarding every other itinerary in the + response. Returning '' instead keeps the row: the itinerary, its flights + and its award overlay are all still useful with the cash price shown as + unavailable. + """ + if fr.price is None: + return "" currency = fr.currency or "USD" return f"{currency}{fr.price:.2f}" @@ -142,8 +154,8 @@ def fli_results_to_search_result(results: Sequence[Any]) -> SearchResult: itinerary=ItineraryDetails(slices=slices, carriers=[]), ), ) - p: float = first_fr.price - if cheapest_price is None or p < cheapest_price: + p: float | None = first_fr.price + if p is not None and (cheapest_price is None or p < cheapest_price): cheapest_price = p cheapest_currency = first_fr.currency or "USD" diff --git a/src/flight_cli/pp/match.py b/src/flight_cli/pp/match.py index 48ebd19..43d10ab 100644 --- a/src/flight_cli/pp/match.py +++ b/src/flight_cli/pp/match.py @@ -3,13 +3,24 @@ Primary key: (normalized first-segment flight number, ISO departure date). Same key can appear at most once per side per day, so a dict-lookup is enough. -Codeshare-fallback key: (origin, destination, departure datetime to minute). -Matrix returns codeshares under the *marketing* flight number (e.g. AA6939 -JFK→LHR), while PointsPath returns the same physical aircraft under the -*operating* flight number (e.g. BA174 — surfaced inside the American PP -query, because PP attributes codeshares to the operator). Flight-number -keys can't bridge that, but route+time can: both sources read the same -airline-published schedule, so origin+dest+minute is a near-tight identity. +Codeshare-fallback key: (origin, destination, departure datetime to minute, +operating-carrier partner group). Matrix returns codeshares under the +*marketing* flight number (e.g. AA6939 JFK→LHR), while PointsPath returns the +same physical aircraft under the *operating* flight number (e.g. BA174 — +surfaced inside the American PP query, because PP attributes codeshares to the +operator). Flight-number keys can't bridge that, but route+time can: both +sources read the same airline-published schedule. + +Route+time alone is NOT an identity, though. On a dense domestic route two +carriers routinely schedule different metal off the same airport at the same +minute — MSY→MIA 2026-09-09T10:45 carries both AA3539 and DL1424. Joining on +route+time alone attached Delta's 9.1k SkyMiles price to the American +itinerary, a redemption that cannot exist (no DL/AA interline award +agreement). So the fallback additionally requires the two carriers to be +plausibly the same metal: identical IATA prefix, or co-members of an alliance +/ bilateral partnership (`_PARTNER_GROUPS`). Identical-carrier is the common +case; the partner check is what keeps the genuine AA-marketed/BA-operated +codeshare joining. The join is provider-neutral: it takes a flat `list[AwardFlight]` (each provider produces these from its own raw shape — see providers/base.py) @@ -21,20 +32,332 @@ from dataclasses import dataclass, field from typing import TYPE_CHECKING +import structlog + if TYPE_CHECKING: - from ..models import Itinerary, SearchResult + from collections.abc import Sequence + + from ..models import Itinerary, SearchResult, Slice from ..providers.base import AwardFlight -MatchKey = tuple[str, str] # (FLIGHT_NUMBER_UPPER_NOSPACE, "YYYY-MM-DD") -RouteTimeKey = tuple[str, str, str] # (ORIGIN_UPPER, DEST_UPPER, "YYYY-MM-DDTHH:MM") +if TYPE_CHECKING: + from structlog.stdlib import BoundLogger + +log: BoundLogger = structlog.get_logger(__name__) # pyright: ignore[reportAny] + +# (FLIGHT_NUMBER_UPPER_NOSPACE, "YYYY-MM-DD", ORIGIN_UPPER, DEST_UPPER) +MatchKey = tuple[str, str, str, str] +# (ORIGIN_UPPER, DEST_UPPER, "YYYY-MM-DDTHH:MM") — carrier is checked +# separately (see `same_metal`), not folded into the key, because a +# codeshare's two carriers differ by design and a dict key can't express +# "equal OR partnered". +RouteTimeKey = tuple[str, str, str] _ISO_MINUTE_LEN = 16 # "YYYY-MM-DDTHH:MM" +_IATA_PREFIX_LEN = 2 +_MIN_MULTI_SEGMENT = 2 # a slice needs 2+ flights before later segments can disagree + +# Carriers that may appear as marketing/operating counterparts for the same +# physical flight. Alliance membership plus the bilateral JVs and equity +# partnerships that actually produce codeshares on routes we search. Keyed by +# IATA code; membership is symmetric (a shared group ⇒ plausibly same metal). +# +# This gates the route+time fallback ONLY. The (flight#, date) primary key is +# unaffected — an exact flight-number hit needs no carrier corroboration. +# Being absent here costs a codeshare match; being wrongly present costs a +# fabricated award price, which is the failure this table exists to prevent. +# Erring toward omission is deliberate. +_PARTNER_GROUPS: tuple[frozenset[str], ...] = ( + # oneworld + frozenset({"AA", "AS", "BA", "AY", "IB", "JL", "MH", "QF", "QR", "RJ", "UL", "CX"}), + # Star Alliance + frozenset( + { + "UA", + "AC", + "LH", + "OS", + "LX", + "SN", + "SK", + "TP", + "TK", + "NH", + "OZ", + "SQ", + "TG", + "NZ", + "ET", + "MS", + "AV", + "CM", + "ZH", + "CA", + "A3", + "BR", + }, + ), + # SkyTeam + frozenset({"DL", "AF", "KL", "AZ", "AM", "KE", "MU", "CZ", "SU", "VN", "RO", "UX", "GA"}), + # Non-alliance bilaterals that codeshare heavily + frozenset({"DL", "VS"}), # Delta / Virgin Atlantic JV + frozenset({"DL", "WS"}), # Delta / WestJet + frozenset({"AA", "GF"}), # American / Gulf Air + frozenset({"AS", "B6"}), # Alaska / JetBlue (Northeast Alliance remnant) + frozenset({"UA", "EI"}), # United / Aer Lingus + frozenset({"EK", "QF"}), # Emirates / Qantas +) + +# Mainline → the regional carriers that operate its metal under its flight +# numbers. This is the DOMINANT real codeshare shape (LH9498 marketed / +# EN8858 operated is the repo's own documented example — see +# docs/memories/gf_routing_and_carriers.md), and an alliance table misses it +# entirely: a feeder is not an alliance member. +# +# Directional and asymmetric ON PURPOSE. Folding these into the symmetric +# `_PARTNER_GROUPS` would make MQ and OH (two unrelated AA regionals) match +# each other. Lookup is `award_carrier in _REGIONAL_OPERATORS[cash_carrier]`, +# so mainline-marketed / regional-operated joins and nothing else does. +# Last verified: 2026-07. +_REGIONAL_OPERATORS: dict[str, frozenset[str]] = { + "AA": frozenset({"MQ", "OH", "YX", "PT", "ZW", "G7", "9K"}), + "DL": frozenset({"9E", "OO", "YX", "G7", "EM"}), + "UA": frozenset({"OO", "YX", "ZW", "C5", "AX", "G7", "EM"}), + "AS": frozenset({"QX", "OO"}), + "LH": frozenset({"EN", "CL", "WK", "EW", "VL"}), + "AF": frozenset({"A5", "XK"}), + "BA": frozenset({"CJ", "SN"}), + "AC": frozenset({"QK", "RV", "ZX", "8P"}), +} def _norm_fn(fn: str | None) -> str: return (fn or "").upper().replace(" ", "") +def _carrier(fn: str | None) -> str: + """IATA carrier prefix of a flight number ('AA3539' → 'AA'). + + Returns '' when the input is missing or too short to carry one, which + `same_metal` treats as unknown-and-therefore-not-joinable. + """ + n = _norm_fn(fn) + return n[:_IATA_PREFIX_LEN] if len(n) > _IATA_PREFIX_LEN else "" + + +def same_carrier(cash_fn: str | None, award_fn: str | None) -> bool: + """Exact IATA-prefix equality, with '' (unparseable) never matching.""" + c, a = _carrier(cash_fn), _carrier(award_fn) + return bool(c) and c == a + + +def same_metal(cash_fn: str | None, award_fn: str | None) -> bool: + """Could these two flight numbers denote the same physical aircraft? + + True when the carriers are identical (the ordinary case — both sides + reporting the same flight), when one is the other's regional operator + (`_REGIONAL_OPERATORS` — the dominant real codeshare shape), or when they + are co-members of `_PARTNER_GROUPS`. False when either carrier is + unparseable: an unknown carrier can't be corroborated, and a wrong join + fabricates an unbookable award price. + + NOTE: this is a *candidate* predicate, deliberately loose. Partner + agreement does not mean same aircraft — two oneworld carriers routinely + fly the same route at the same minute on different metal. `_pick_metal` + is what resolves a bucket of candidates down to the right one; calling + `same_metal` alone as an accept/reject test reintroduces the collision + (cash AA118 + award AS17 at the same minute are both "same metal" here). + """ + c, a = _carrier(cash_fn), _carrier(award_fn) + if not c or not a: + return False + if c == a: + return True + if a in _REGIONAL_OPERATORS.get(c, frozenset()): + return True + return any(c in g and a in g for g in _PARTNER_GROUPS) + + +def _slice_stop_count(s: Slice) -> int: + """Number of connections on a cash slice. `stops` is authoritative when + populated; otherwise derive from the segment count.""" + if s.stops: + return len(s.stops) + return max(len(s.flights) - 1, 0) + + +def _pick_metal( + cash_fn: str, + cash_arrival: str, + candidates: list[AwardFlight], +) -> list[AwardFlight]: + """Resolve one route+time bucket to the awards that are the cash flight's + own metal. + + A bucket keyed on (origin, destination, departure minute) can hold several + genuinely different aircraft — same-alliance carriers compete head-to-head + on dense routes, so AA118 and AS17 can both leave JFK for LAX at 10:45. + Accepting every partner in the bucket is what let a 7.5k Alaska price + render on the American row. + + Resolution, in order: + 1. **Same carrier wins.** A same-carrier award at this route+minute IS + the flight; partners beside it are different metal. + 2. **Single partner.** Otherwise the bucket is codeshare-shaped (the + fallback's reason for existing: marketing AA6939 ↔ operating BA174). + Accept only when the survivors resolve to ONE carrier — two distinct + partner carriers means we cannot tell which is the metal. + + Arrival time narrows WITHIN each stage rather than ahead of them (see + `_narrow`). Ordering matters and getting it wrong is not a near-miss: + filtering the whole bucket by arrival first lets a wrong-carrier award + whose arrival happens to match survive while the correct same-carrier + award — one that merely omits its arrival — is deleted before the + carrier rule ever sees it. That renders another aircraft's price on the + row, which is the exact defect this function exists to prevent. + """ + # Stage 1 requires the FULL flight number, not just the carrier prefix. + # `same_carrier` only compares the IATA prefix, so treating that as "this + # IS the flight" let AA999 claim exact-match priority over cash AA867 and + # skip straight to `_narrow`, which admits a missing arrival when nothing + # matched exactly. A different number on the same carrier is a different + # flight; it belongs in the partner stage where ambiguity is judged. + exact = [af for af in candidates if _norm_fn(af.flight_number) == _norm_fn(cash_fn)] + if exact: + return _narrow(exact, cash_arrival) + partners = [af for af in candidates if same_metal(cash_fn, af.flight_number)] + # The hand-curated tables are the weakest evidence in this module, and by + # construction they only decide a match when nothing stronger applied. + # That makes a stale entry fail SILENTLY — the wrong price just appears. + # Log every table-arbitrated match so the failure mode is greppable, and + # so we can tell from real traffic whether these tables still earn their + # keep (measured 2026-07: 0 of 471 live awards lacked an arrival, i.e. + # this path effectively never fires today). + for af in partners: + if not same_carrier(cash_fn, af.flight_number): + log.debug( + "award_match_via_partner_table", + cash_flight=cash_fn, + award_flight=af.flight_number, + program=af.program, + ) + # A DIFFERENT number on the SAME carrier is simply a different flight — + # an airline does not sell one departure under two of its own numbers. + # Only a genuine codeshare (different carrier) explains a number mismatch, + # so a same-carrier candidate that reached this stage must prove itself + # with a matching arrival; absence of evidence is not enough for it. + partners = [ + af + for af in partners + if not same_carrier(cash_fn, af.flight_number) + or (cash_arrival and _iso_minute(af.arrival) == cash_arrival) + ] + # Arrival is used in two distinct ways here, and conflating them is a bug. + # Eliminating a candidate whose arrival CONTRADICTS the cash flight is + # sound — that is positive evidence of different metal. Letting arrival + # *choose* between carriers is not: a partner that merely omits its + # arrival would lose to an unrelated carrier that happens to publish a + # matching one. So contradicted candidates go first, then ambiguity is + # judged among the survivors. + partners = _drop_contradicted(partners, cash_arrival) + if not partners: + return [] + if len({_carrier(af.flight_number) for af in partners}) > 1: + return [] + return _narrow(partners, cash_arrival) + + +def _by_journey_shape( + candidates: list[AwardFlight], + cash_flights: list[str], + cash_stop_airports: list[str], +) -> list[AwardFlight]: + """Drop candidates whose journey shape contradicts the cash slice's. + + Every key in this module identifies a journey by its FIRST segment, so two + journeys that share a first flight and diverge afterwards collapse + together. Both are real in live data: seats.aero returns "AA1444, BA216" + and "AA1444, AA100" on one JFK→LHR date, and PointsPath returns four + distinct AA1650 MSY→LHR journeys sharing a 12:22 departure and one stop, + separable only by where they connect and when they land. + + Two independent signals, because no single one is available everywhere: + + * **Connection airports** — Matrix fills `Slice.stops` and BOTH award + providers supply the equivalent, so this is the cross-provider check. + Only compared when the cash side is a connection and the candidate says + something; empty on the award side is "no evidence", not agreement. + * **Segment flight numbers** — stronger (it pins each leg, not just the + hub) but seats.aero-only; PointsPath sends the first number alone. + + Each is skipped when its evidence is absent, so a provider that supplies + neither is judged by the carrier and arrival rules as before — no match is + lost that used to succeed. + """ + if len(cash_flights) < _MIN_MULTI_SEGMENT and not cash_stop_airports: + return candidates # nonstop: nothing past segment 0 to contradict + want_stops = [c.upper() for c in cash_stop_airports] + want_flights = [_norm_fn(f) for f in cash_flights] + out: list[AwardFlight] = [] + for af in candidates: + if want_stops and af.stop_airports and [c.upper() for c in af.stop_airports] != want_stops: + continue + if ( + len(want_flights) >= _MIN_MULTI_SEGMENT + and af.segment_flight_numbers + and [_norm_fn(f) for f in af.segment_flight_numbers] != want_flights + ): + continue + out.append(af) + return out + + +def _drop_contradicted(candidates: list[AwardFlight], cash_arrival: str) -> list[AwardFlight]: + """Remove candidates whose arrival positively disagrees with the cash + flight's. Missing arrivals survive — absence of evidence is not + contradiction. Unlike `_narrow` this never *selects* a winner, so it is + safe to run before carrier ambiguity is judged.""" + if not cash_arrival: + return candidates + return [af for af in candidates if _iso_minute(af.arrival) in ("", cash_arrival)] + + +def _narrow(candidates: list[AwardFlight], cash_arrival: str) -> list[AwardFlight]: + """Keep only the candidates arriving at the cash flight's arrival minute. + + Two aircraft sharing a departure minute on the same route essentially never + share an arrival minute too, so this separates same-carrier rotations the + carrier rules cannot. Measured on a live MSY→MIA/FLL payload: route+departure + alone left 3 multi-carrier buckets of 41; adding arrival left 0 of 91. + + Per-candidate, not all-or-nothing. A *disagreeing* arrival is evidence of + different metal and is always dropped, even when that empties the bucket. + Judging each candidate on its own data avoids two failure modes an + all-or-nothing filter has — one member missing an arrival disabling the + filter for the whole bucket, and a bucket where every arrival disagrees + (the strongest possible evidence) being restored wholesale. + + Candidates with NO arrival are the awkward middle: absence of evidence, not + evidence of absence. They are admitted only when nothing in the bucket + matched exactly. Otherwise a cheaper no-arrival sibling would sit beside + the award that positively confirmed the cash flight, and the renderer's + lowest-miles pick would show the sibling's price — silently preferring the + unverified candidate over the verified one. + + Note both sides express local time at the airport: Matrix sends an offset + ('...T09:04-04:00'), PointsPath naive ('...T09:04:00'), seats.aero a + misleading 'Z' on what is also local. `_iso_minute` truncates all three to + the same wall-clock key. If a caller ever starts genuinely parsing these + as instants, seats.aero's fake 'Z' becomes a real ~offset-sized bug. + """ + if not cash_arrival: + return candidates + exact = [af for af in candidates if _iso_minute(af.arrival) == cash_arrival] + if exact: + return exact + return [af for af in candidates if not _iso_minute(af.arrival)] + + def _iso_date(s: str | None) -> str: """Best-effort isolate the YYYY-MM-DD prefix from various formats.""" if not s: @@ -54,39 +377,60 @@ def _iso_minute(s: str | None) -> str: return s[:_ISO_MINUTE_LEN] if len(s) >= _ISO_MINUTE_LEN else "" +def _cash_slice(it: Itinerary, slice_index: int) -> Slice | None: + """Bounds-checked slice access — the guard every cash_* key repeats.""" + itn = it.itinerary + if not itn or not itn.slices or slice_index >= len(itn.slices): + return None + return itn.slices[slice_index] + + def cash_match_key(it: Itinerary, slice_index: int = 0) -> MatchKey | None: """Build the match key from a Matrix itinerary's slice's first flight. Default slice_index=0 = outbound leg. For round-trips pass 1 to match the return leg; for multi-city pass 2, 3, etc. + + Route is part of the key: a flight number is only unique *per route* on a + given date. Airlines reuse numbers across the day's rotations, so without + origin/dest an `AA100 JFK→LHR 18:00` cash row matched an `AA100 MIA→DFW + 06:30` award — same number, same date, different flight. """ - itn = it.itinerary - if not itn or not itn.slices or slice_index >= len(itn.slices): + s = _cash_slice(it, slice_index) + if s is None: return None - s = itn.slices[slice_index] flights = s.flights or [] if not flights: return None fn = _norm_fn(flights[0]) dep = _iso_date(s.departure) - if not fn or not dep: + o = ((s.origin.code if s.origin else None) or "").upper() + d = ((s.destination.code if s.destination else None) or "").upper() + if not fn or not dep or not o or not d: return None - return (fn, dep) + return (fn, dep, o, d) -def award_match_key(af: AwardFlight) -> MatchKey: - return (_norm_fn(af.flight_number), _iso_date(af.departure)) +def award_match_key(af: AwardFlight) -> MatchKey | None: + o = (af.origin or "").upper() + d = (af.destination or "").upper() + fn = _norm_fn(af.flight_number) + dep = _iso_date(af.departure) + if not fn or not dep or not o or not d: + return None + return (fn, dep, o, d) def cash_route_time_key(it: Itinerary, slice_index: int = 0) -> RouteTimeKey | None: """Codeshare-fallback key: origin, destination, minute-precision departure. Doesn't require `slice.flights` to be populated — origin/dest/departure - are enough to anchor the same physical flight on the award side.""" - itn = it.itinerary - if not itn or not itn.slices or slice_index >= len(itn.slices): + are enough to anchor a candidate. The carrier corroboration that makes + the candidate a *match* is applied separately in `join` (`same_metal`), + which does need `flights[0]`.""" + s = _cash_slice(it, slice_index) + if s is None: return None - s = itn.slices[slice_index] o = ((s.origin.code if s.origin else None) or "").upper() d = ((s.destination.code if s.destination else None) or "").upper() t = _iso_minute(s.departure) @@ -95,6 +439,15 @@ def cash_route_time_key(it: Itinerary, slice_index: int = 0) -> RouteTimeKey | N return (o, d, t) +def cash_first_flight_number(it: Itinerary, slice_index: int = 0) -> str: + """First marketing flight number on the slice, '' when absent.""" + s = _cash_slice(it, slice_index) + if s is None: + return "" + flights = s.flights or [] + return _norm_fn(flights[0]) if flights else "" + + def award_route_time_key(af: AwardFlight) -> RouteTimeKey | None: o = (af.origin or "").upper() d = (af.destination or "").upper() @@ -123,7 +476,7 @@ def cash_matched_id_key(it: Itinerary, slice_index: int = 0) -> str | None: return fid or None -def join( # noqa: PLR0912 — three index lookups in priority order, hard to split cleanly +def join( search: SearchResult, awards: list[AwardFlight], *, @@ -131,18 +484,27 @@ def join( # noqa: PLR0912 — three index lookups in priority order, hard to sp ) -> list[MatchedFare]: """Outer-join cash itineraries onto award flights. - Match strategy, in priority order: + Match strategy. Keys 1 and 2 are exact identities; key 3 is a heuristic + that must be resolved, not merely filtered: 1. **Matched-ID** (`flight_id` ↔ `matched_google_flight_id`). Exact string equality on PP's echoed `matchedGoogleFlightId`. Fires only when the cash side has `flight_id` (gflight backend) AND the provider was called with `enable_matching=True` + cash hints. - 2. **(flight#, date)** primary heuristic key. - 3. **(route, time)** codeshare fallback (Matrix's marketing flight# - won't equal PP's operating flight#, but origin+dest+minute identifies - the same physical flight). + 2. **(flight#, date, origin, dest)** primary key. Route is part of the + identity — flight numbers are only unique per route per day. + 3. **(route, time) → `_pick_metal`** codeshare fallback. Matrix's + marketing flight# won't equal PP's operating flight#, so + origin+dest+minute anchors the *bucket* — but a bucket can hold + several genuinely different aircraft (MSY→MIA 10:45 held both + AA3539 and DL1424; JFK→LAX 10:45 holds both AA118 and AS17). + `_pick_metal` resolves the bucket: same-carrier wins outright, and + an ambiguous multi-partner bucket resolves to nothing. Hits across all three are unioned and deduped by AwardFlight identity, so - a flight satisfying multiple keys isn't double-attached. + a flight satisfying multiple keys isn't double-attached. Every attached + award must also agree with the cash slice on connection count — a 1-stop + award is cheaper than the nonstop it would be rendered beside, so it wins + the renderer's lowest-miles pick and prints under a "nonstop" label. Cash itineraries with no award match keep an empty `awards` list — caller decides whether to render them or filter to inner-join. @@ -160,7 +522,7 @@ def join( # noqa: PLR0912 — three index lookups in priority order, hard to sp if af.matched_google_flight_id: mid_idx.setdefault(af.matched_google_flight_id, []).append(af) fn_k = award_match_key(af) - if fn_k[0]: + if fn_k: fn_idx.setdefault(fn_k, []).append(af) rt_k = award_route_time_key(af) if rt_k: @@ -168,30 +530,64 @@ def join( # noqa: PLR0912 — three index lookups in priority order, hard to sp out: list[MatchedFare] = [] for it in search.solutions: - matched: list[AwardFlight] = [] - seen_ids: set[int] = set() - + s = _cash_slice(it, slice_index) + cash_stops = _slice_stop_count(s) if s else 0 + + cash_fn = cash_first_flight_number(it, slice_index=slice_index) + cash_arr = _iso_minute(s.arrival) if s else "" + cash_flights = list(s.flights or []) if s else [] + cash_stop_airports = [(e.code or "") for e in (s.stops or [])] if s else [] + + # Gather the raw candidates from every applicable key FIRST, then run + # one resolution pass over the union. + # + # Resolving each bucket separately and unioning the winners is wrong: + # the resolver's job is to decide among competing claims, so a bucket + # that only sees part of the field decides on partial information. A + # concrete failure — cash AA6939 with a BA174 award (route+time) and an + # AS99 award (matched-ID): route+time sees two different partner + # carriers, correctly calls it ambiguous and yields nothing, but the + # matched-ID bucket sees AS99 alone, calls it an unambiguous single + # partner, and admits it. The union then renders 7.5k Alaska on an + # American row — precisely the rejection one bucket had just made. + # + # The keys are *discovery* mechanisms; resolution is a single judgment + # over everything they found. + # `mid`: PP mints this ID from a hint we supplied and its own matcher + # is documented as loose, so an echoed ID is a claim, not proof. + # `fn`: (flight#, date, route) keys on segment 0 only, so connecting + # journeys sharing a first flight collapse together. + # `rt`: route+departure-minute; two carriers can share both. + # None of the three is an identity on its own. + hits: list[Sequence[AwardFlight]] = [] mid_k = cash_matched_id_key(it, slice_index=slice_index) - if mid_k and mid_k in mid_idx: - for af in mid_idx[mid_k]: - if id(af) in seen_ids: - continue - seen_ids.add(id(af)) - matched.append(af) + if mid_k: + hits.append(mid_idx.get(mid_k, ())) fn_k = cash_match_key(it, slice_index=slice_index) - if fn_k and fn_k in fn_idx: - for af in fn_idx[fn_k]: - if id(af) in seen_ids: - continue - seen_ids.add(id(af)) - matched.append(af) + if fn_k: + hits.append(fn_idx.get(fn_k, ())) rt_k = cash_route_time_key(it, slice_index=slice_index) - if rt_k and rt_k in rt_idx: - for af in rt_idx[rt_k]: - if id(af) in seen_ids: - continue - seen_ids.add(id(af)) - matched.append(af) - + if rt_k: + hits.append(rt_idx.get(rt_k, ())) + + raw: list[AwardFlight] = [] + seen_raw: set[int] = set() + for hit in hits: + for af in hit: + if id(af) not in seen_raw: + seen_raw.add(id(af)) + raw.append(af) + + # Connection count is an objective property of the journey, so drop + # mismatches BEFORE resolution — otherwise an ineligible candidate can + # make the field look ambiguous and suppress a valid codeshare that + # would have won on its own. + raw = [af for af in raw if af.num_connections == cash_stops] + + matched = _pick_metal( + cash_fn, + cash_arr, + _by_journey_shape(raw, cash_flights, cash_stop_airports), + ) out.append(MatchedFare(itinerary=it, awards=matched)) return out diff --git a/src/flight_cli/pp/models.py b/src/flight_cli/pp/models.py index e038892..b2cdc41 100644 --- a/src/flight_cli/pp/models.py +++ b/src/flight_cli/pp/models.py @@ -62,12 +62,18 @@ class OutboundFlight(_Loose): firstFlightNumber: str googleAirlineName: str | None = None numConnections: int = 0 + # Connection airport codes in order, e.g. ["DFW"]. Empty for a nonstop. + # PP has always sent this; `extra="ignore"` was silently dropping it. It's + # the only journey-shape signal PP gives beyond the first flight number, + # and Matrix populates the comparable `Slice.stops`. + stops: list[str] = [] externalId: str | None = None matchedGoogleFlightId: str | None = None matchedGoogleFlightCashPriceUsd: float | None = None perCabinMilesPricing: list[PerCabinMilesPricing] = [] _none_pricing = field_validator("perCabinMilesPricing", mode="before")(_none_to_empty_list) + _none_stops = field_validator("stops", mode="before")(_none_to_empty_list) class AirlineSearchResponse(_Loose): diff --git a/src/flight_cli/providers/base.py b/src/flight_cli/providers/base.py index c986964..e24b5ba 100644 --- a/src/flight_cli/providers/base.py +++ b/src/flight_cli/providers/base.py @@ -46,6 +46,11 @@ class CabinAward: tax_usd: float tax_currency: str is_basic_economy: bool | None = None + # Seats the provider says are left at this price, when it says. None means + # "not reported" — NOT "none available"; PointsPath doesn't expose it and + # seats.aero's value is often 0 through staleness rather than sell-out. + # The renderer uses it only to flag an award that cannot seat the party. + remaining_seats: int | None = None @dataclass @@ -67,6 +72,25 @@ class AwardFlight: arrival: str flight_number: str num_connections: int = 0 + # Every segment's marketing flight number, in order, when the provider + # supplies them (seats.aero does; PointsPath returns only the first). + # `flight_number` is segment 0, so two journeys that share a first segment + # and diverge afterwards are indistinguishable without this — seats.aero + # returns both "AA1444, BA216" and "AA1444, AA100" on one route+date, and + # collapsing them lets the cheaper journey's price render on the other's + # row. Empty when the provider can't say, which the matcher treats as + # "no segment evidence" rather than agreement. + segment_flight_numbers: list[str] = field(default_factory=list[str]) + # Connection airport codes in order (["DFW"]); empty for a nonstop OR when + # the provider doesn't say. Distinguishing those two states is the caller's + # job — see `_by_journey_shape`, which pairs this with `num_connections`. + # + # This is the ONLY journey-shape signal PointsPath gives beyond the first + # flight number, and Matrix populates the directly comparable + # `Slice.stops`, so it works cross-provider where segment numbers (which + # only seats.aero sends) do not. Live MSY->LHR has four distinct AA1650 + # journeys sharing a departure minute and connection count. + stop_airports: list[str] = field(default_factory=list[str]) # provider/program metadata — used for rendering only provider: str = "" # display name, e.g. "PointsPath", "seats.aero" diff --git a/src/flight_cli/providers/pointspath/provider.py b/src/flight_cli/providers/pointspath/provider.py index fc7af6e..5f43771 100644 --- a/src/flight_cli/providers/pointspath/provider.py +++ b/src/flight_cli/providers/pointspath/provider.py @@ -74,6 +74,7 @@ def _flight_to_award( arrival=of.localArrivalDateTime, flight_number=of.firstFlightNumber, num_connections=of.numConnections, + stop_airports=[c.upper() for c in of.stops], provider="PointsPath", program=program, miles_to_cash_ratio=miles_to_cash_ratio, diff --git a/src/flight_cli/providers/seats_aero/models.py b/src/flight_cli/providers/seats_aero/models.py index 9047818..93591c7 100644 --- a/src/flight_cli/providers/seats_aero/models.py +++ b/src/flight_cli/providers/seats_aero/models.py @@ -57,7 +57,11 @@ class SeatsAvailabilityTrip(_Loose): OriginAirport: str DestinationAirport: str - DepartsAt: str # ISO 8601 UTC, e.g. "2026-08-15T06:30:00Z" + # MISLABELLED UPSTREAM: carries a 'Z' suffix but the value is LOCAL time + # at the airport, not UTC. Verified against this repo's own fixture — + # honouring the Z gives ~12.1h JFK→LHR nonstops against a real ~7h. + # `provider._local_naive` strips the suffix on the way into AwardFlight. + DepartsAt: str # naive local despite the 'Z', e.g. "2026-08-15T06:30:00Z" ArrivesAt: str Cabin: str # lowercase: "economy", "premium", "business", "first" FlightNumbers: str # comma list, in segment order diff --git a/src/flight_cli/providers/seats_aero/provider.py b/src/flight_cli/providers/seats_aero/provider.py index 0ddba75..7b02c23 100644 --- a/src/flight_cli/providers/seats_aero/provider.py +++ b/src/flight_cli/providers/seats_aero/provider.py @@ -68,14 +68,30 @@ # Seats.aero cabin slug → CabinAward.cabin string. Aligns with PointsPath's # "Economy"/"Business"/"First"/"Premium" labels so the renderer doesn't # have to disambiguate by provider. +# Values MUST match the CLI's canonical cabin vocabulary (`_CABIN_ALIASES` in +# pp/cli.py), because the renderer selects awards by exact cabin-string +# equality. "Premium" did not equal the canonical "Premium economy", so every +# seats.aero premium award silently vanished from that column. _CABIN_LABELS: dict[str, str] = { "economy": "Economy", - "premium": "Premium", + "premium": "Premium economy", "business": "Business", "first": "First", } +# Canonical cabin label -> seats.aero's own query slug. The outbound filter +# needs the inverse of `_CABIN_LABELS`, and a bare `.lower()` no longer works +# now that the canonical name is "Premium economy": the API expects "premium", +# so lowercasing sent "premium economy" and the filter silently matched +# nothing. +_CABIN_SLUGS: dict[str, str] = {label: slug for slug, label in _CABIN_LABELS.items()} + + +def _cabin_slug(label: str) -> str: + return _CABIN_SLUGS.get(label, label.lower()) + + def _program_label(slug: str) -> str: return _PROGRAM_LABELS.get(slug, slug.title()) @@ -84,6 +100,33 @@ def _cabin_label(slug: str) -> str: return _CABIN_LABELS.get(slug.lower(), slug.title()) +def _segment_flight_numbers(flight_numbers: str) -> list[str]: + """`"AA4671, BA216"` -> `["AA4671", "BA216"]` — every segment, in order. + + The matcher needs the whole list to tell apart journeys that share a first + segment: seats.aero returns both "AA1444, BA216" and "AA1444, AA100" on one + JFK->LHR date, and keying on segment 0 alone collapses them. + """ + return [n.strip().upper().replace(" ", "") for n in flight_numbers.split(",") if n.strip()] + + +def _local_naive(ts: str) -> str: + """Strip seats.aero's spurious 'Z' suffix. + + Their `DepartsAt`/`ArrivesAt` are labelled UTC but carry LOCAL time at each + airport. Fixture arithmetic proves it: JFK→LHR nonstops read ~12.1h block + time when the Z is honoured, against a real ~7h — and 09:35 local JFK plus + 7h is 21:35 London, which is the 21:40 they report. + + The matcher compares these against Matrix cash times, which are local too, + so passing the Z through would be a latent ~offset-sized error the moment + anything parses these as instants instead of truncating the suffix away. + Normalizing here keeps `AwardFlight.departure`/`.arrival` meaning one + thing — naive local — across every provider. + """ + return ts.removesuffix("Z") + + def _first_flight_number(flight_numbers: str) -> str: """`"AA4671, BA216"` → `"AA4671"`. The matcher keys on the first marketing flight number, same convention as PointsPath. Multi-segment @@ -119,14 +162,19 @@ def _group_trips_to_awards( miles=t.MileageCost, tax_usd=t.TotalTaxes / 100.0, # seats.aero returns cents tax_currency=tax_currency, + # 0 is seats.aero's "unknown/stale", not a hard zero — see the + # field comment in models.py — so don't turn it into a claim. + remaining_seats=t.RemainingSeats or None, ) if af is None: grouped[key] = AwardFlight( origin=t.OriginAirport, destination=t.DestinationAirport, - departure=t.DepartsAt, - arrival=t.ArrivesAt, + departure=_local_naive(t.DepartsAt), + arrival=_local_naive(t.ArrivesAt), flight_number=_first_flight_number(t.FlightNumbers), + segment_flight_numbers=_segment_flight_numbers(t.FlightNumbers), + stop_airports=[c.upper() for c in t.Connections], num_connections=t.Stops, provider="Seats.aero", program=_program_label(t.Source), @@ -220,7 +268,7 @@ async def search_leg( usual. """ _ = num_passengers, cash_hints - cabin_slugs = tuple(c.lower() for c in cabins) if cabins else None + cabin_slugs = tuple(_cabin_slug(c) for c in cabins) if cabins else None try: page = await self._client.search( origin=leg.origin, diff --git a/tests/fixtures/matrix_url/spa_routing_jfk_lhr.txt b/tests/fixtures/matrix_url/spa_routing_jfk_lhr.txt new file mode 100644 index 0000000..02d903c --- /dev/null +++ b/tests/fixtures/matrix_url/spa_routing_jfk_lhr.txt @@ -0,0 +1,46 @@ +# Captured 2026-08-01 by driving the real Matrix SPA (patchright + real +# Chrome), one-way JFK->LHR with Routing='BA+' and Extension='MAXSTOPS 0' +# set in Advanced controls. This is the shape the app writes into its own +# URL when routing codes ARE set; the other fixtures in this directory are +# captures with none set, where the SPA omits the four keys entirely. +# +# Note the names differ from the /batch API, which calls the same values +# routeLanguage / commandLine. + +{ + "type": "one-way", + "slices": [ + { + "origin": [ + "JFK" + ], + "dest": [ + "LHR" + ], + "routing": "BA+", + "ext": "MAXSTOPS 0", + "routingRet": "", + "extRet": "", + "dates": { + "searchDateType": "specific", + "departureDate": "2026-08-10", + "departureDateType": "depart", + "departureDateModifier": "0", + "departureDatePreferredTimes": [], + "returnDateType": "depart", + "returnDateModifier": "0", + "returnDatePreferredTimes": [] + } + } + ], + "options": { + "cabin": "COACH", + "stops": "-1", + "extraStops": "1", + "allowAirportChanges": "true", + "showOnlyAvailable": "true" + }, + "pax": { + "adults": "1" + } +} diff --git a/tests/pp/test_auth_login.py b/tests/pp/test_auth_login.py index 9d00222..4846f4f 100644 --- a/tests/pp/test_auth_login.py +++ b/tests/pp/test_auth_login.py @@ -10,6 +10,7 @@ import base64 import json +import os from typing import TYPE_CHECKING, Any import pytest @@ -172,3 +173,60 @@ def chrome(domains: list[str]) -> list[_Cookie]: t = login_from_chrome() assert t.user_email == "chrome@example.com" assert seen_domains == [["pointspath.com"]], "Should scope read to pointspath.com only" + + +# ───────── the token file is never briefly world-readable ───────── + + +def test_token_file_is_created_0600_not_widened_then_tightened( + tmp_path: pathlib.Path, monkeypatch: Any +) -> None: + """`write_text` creates at the process umask (0644 by default), so writing + then chmod'ing left a bearer token to a paid account world-readable for the + window between the two calls. The file must be created 0600.""" + import stat + + from flight_cli.pp import auth as auth_mod + + monkeypatch.setattr(auth_mod, "CONFIG_DIR", tmp_path) + monkeypatch.setattr(auth_mod, "TOKENS_PATH", tmp_path / "pp.json") + + modes: list[int] = [] + real_open = os.open + + def spy(path: Any, flags: int, mode: int = 0o777, **kw: Any) -> int: + if str(path).endswith("pp.json"): + modes.append(mode) + return real_open(path, flags, mode, **kw) + + monkeypatch.setattr(os, "open", spy) + auth_mod.save_tokens( + auth_mod.Tokens( + access_token="A", # noqa: S106 — dummy test value + refresh_token="R", # noqa: S106 — dummy test value + expires_at=9_999_999_999, + user_email="x@y.z", + ), + ) + # Requested at creation, not applied afterwards. + assert modes == [0o600] + final = stat.S_IMODE((tmp_path / "pp.json").stat().st_mode) + assert final == 0o600 + + +def test_saved_tokens_round_trip(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + from flight_cli.pp import auth as auth_mod + + monkeypatch.setattr(auth_mod, "CONFIG_DIR", tmp_path) + monkeypatch.setattr(auth_mod, "TOKENS_PATH", tmp_path / "pp.json") + original = auth_mod.Tokens( + access_token="ACCESS", # noqa: S106 — dummy test value + refresh_token="REFRESH", # noqa: S106 — dummy test value + expires_at=9_999_999_999, + user_email="x@y.z", + ) + auth_mod.save_tokens(original) + loaded = auth_mod.load_tokens() + assert loaded is not None + assert loaded.access_token == "ACCESS" # noqa: S105 — asserting a dummy round-trip + assert loaded.user_email == "x@y.z" diff --git a/tests/pp/test_cli.py b/tests/pp/test_cli.py index 7889ed1..b7edf7f 100644 --- a/tests/pp/test_cli.py +++ b/tests/pp/test_cli.py @@ -3,18 +3,25 @@ from __future__ import annotations +import json +from typing import Any + import pytest +from flight_cli.models import Itinerary, ItineraryDetails, Slice, SliceEndpoint from flight_cli.pp.cli import ( _best_award_for_cabin, _fmt_award_cell, + _fmt_funding, _fmt_iso_compact, _fmt_stops, _normalize_cabin, _parse_cash, _parse_csv, _render_pp_only, + _serialize_matches, ) +from flight_cli.pp.match import MatchedFare from flight_cli.providers.base import AwardFlight, CabinAward # ───────────────────────────── _parse_cash ───────────────────────────────── @@ -126,7 +133,7 @@ def test_best_award_for_cabin_picks_cheapest_in_miles(): ] best = _best_award_for_cabin(awards, "Economy") assert best is not None - miles, _tax, program, _banks = best + miles, _tax, program, _banks, _ccy, _seats = best assert (miles, program) == (30_000, "Cheap") @@ -229,3 +236,128 @@ def test_award_table_does_not_truncate_program_name(monkeypatch: pytest.MonkeyPa assert "American Airlines" in text # Compact, unambiguous departure time (not raw '2026-08-15T13:53'). assert "Aug15 13:53" in text + + +# ───────── renderer: a cell must be a true statement about its row ───────── + + +def _ra( + program: str, + miles: int, + tax: float = 6.0, + *, + cabin: str = "Economy", + basic: bool | None = None, + banks: list[str] | None = None, +) -> AwardFlight: + return AwardFlight( + origin="JFK", + destination="LHR", + departure="2026-08-15T18:00:00", + arrival="2026-08-16T06:00:00", + flight_number="AA100", + num_connections=0, + provider="PointsPath", + program=program, + miles_to_cash_ratio=0.0125, + funding_banks=banks or ["Chase"], + cabins=[ + CabinAward( + cabin=cabin, + miles=miles, + tax_usd=tax, + tax_currency="USD", + is_basic_economy=basic, + ), + ], + ) + + +def test_equal_miles_tie_breaks_on_cash_out_of_pocket() -> None: + """Ranking on miles alone let provider order decide between a 30k + $500 + offer and an identical 30k + $6 one.""" + cell = _fmt_award_cell([_ra("Pricey", 30000, 500.0), _ra("Cheap", 30000, 6.0)], "Economy") + assert "Cheap" in cell + assert "$6" in cell + + +def test_basic_economy_is_not_rendered_as_unrestricted_economy() -> None: + """A basic-economy award carries different seat, bag and change rights, so + showing it under the plain Economy heading overstates what is bought.""" + unrestricted = _fmt_award_cell([_ra("Real", 9500)], "Economy") + assert "(basic)" not in unrestricted + + basic_only = _fmt_award_cell([_ra("American", 9500, basic=True)], "Economy") + assert "(basic)" in basic_only + + +def test_unrestricted_award_wins_over_a_cheaper_basic_one() -> None: + """The cheaper basic fare must not silently displace a real one.""" + cell = _fmt_award_cell([_ra("Cheap", 5000, basic=True), _ra("Real", 9500)], "Economy") + assert "Real" in cell + assert "(basic)" not in cell + + +def test_funding_column_describes_only_the_displayed_award() -> None: + """Unioning banks across every attached award implied that programs + funding hidden, costlier offers also funded the winning one.""" + awards = [_ra("Amex", 30000, banks=["Amex"]), _ra("Chase", 40000, banks=["Chase"])] + assert _fmt_funding(awards, ("Economy",)) == "Amex" + + +def test_json_return_leg_describes_the_return_slice() -> None: + """`_serialize_matches` hardcoded slices[0], so the `--json` return leg + reported the OUTBOUND flight number and route beside the return leg's + awards, under a label saying "return".""" + it = Itinerary( + displayTotal="USD500.00", + itinerary=ItineraryDetails( + slices=[ + Slice( + flights=["AA100"], + departure="2026-08-15T18:00:00", + origin=SliceEndpoint(code="JFK"), + destination=SliceEndpoint(code="LHR"), + ), + Slice( + flights=["BA200"], + departure="2026-08-22T10:00:00", + origin=SliceEndpoint(code="LHR"), + destination=SliceEndpoint(code="JFK"), + ), + ], + carriers=[], + ), + ) + payload: Any = json.loads(_serialize_matches([MatchedFare(itinerary=it, awards=[])], 1)) + assert payload[0]["flight"] == "BA200" + assert payload[0]["origin"] == "LHR" + assert payload[0]["destination"] == "JFK" + + +def test_non_usd_tax_is_labelled_and_suppresses_cpm() -> None: + """A EUR tax printed as "$" both misstates the amount and invites adding it + to a USD fare. ¢/mi nets tax off USD cash, so a foreign tax would subtract + the wrong magnitude — suppressed rather than converted, since there is no + rate source and a wrong valuation is worse than a missing one.""" + usd = _fmt_award_cell([_ra("American", 30000, 6.0)], "Economy", 500.0) + assert "$6" in usd + assert "¢/mi" in usd + + eur = AwardFlight( + origin="JFK", + destination="LHR", + departure="d", + arrival="a", + flight_number="AA100", + num_connections=0, + provider="X", + program="American", + miles_to_cash_ratio=0.0125, + funding_banks=["Chase"], + cabins=[CabinAward(cabin="Economy", miles=30000, tax_usd=180.0, tax_currency="EUR")], + ) + cell = _fmt_award_cell([eur], "Economy", 500.0) + assert "180 EUR" in cell + assert "$" not in cell + assert "¢/mi" not in cell diff --git a/tests/pp/test_client.py b/tests/pp/test_client.py index f42b301..e90722d 100644 --- a/tests/pp/test_client.py +++ b/tests/pp/test_client.py @@ -3,10 +3,17 @@ from __future__ import annotations import json +import os import pathlib +import time from typing import Any -from flight_cli.pp.client import enabled_airlines +from flight_cli.pp.client import ( + enabled_airlines, + is_unsupported_airline_response, + load_unsupported_airlines, + remember_unsupported_airline, +) from flight_cli.pp.models import PricingInfoResponse FIX = pathlib.Path(__file__).parent / "fixtures" @@ -69,3 +76,156 @@ def test_enabled_returns_pricing_order(): pricing_order = [p.airline for p in _pricing().pricingInfos] expected = [a for a in pricing_order if a in set(enabled)] assert list(enabled) == expected + + +# ───────────── unsupported-airline negative cache (400 spam fix) ───────────── + + +def test_is_unsupported_airline_response_matches_the_real_body() -> None: + """The exact shape PP returns for an airline it doesn't serve.""" + assert is_unsupported_airline_response(400, '{"error":"unsupported airline"}') is True + assert is_unsupported_airline_response(400, '{"error":"Unsupported Airline"}') is True + + +def test_is_unsupported_airline_response_ignores_other_failures() -> None: + """Must stay narrow: a transient or auth failure would otherwise + permanently blacklist a working airline.""" + assert is_unsupported_airline_response(400, '{"error":"bad route"}') is False + assert is_unsupported_airline_response(429, '{"error":"unsupported airline"}') is False + assert is_unsupported_airline_response(500, "") is False + assert is_unsupported_airline_response(503, '{"error":"unsupported airline"}') is False + + +def test_unsupported_cache_roundtrips(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + assert load_unsupported_airlines() == frozenset() + remember_unsupported_airline("ThaiAirways") + remember_unsupported_airline("ANA") + remember_unsupported_airline("ThaiAirways") # idempotent + assert load_unsupported_airlines() == frozenset({"ANA", "ThaiAirways"}) + written: Any = json.loads(cache.read_text()) + assert sorted(written) == ["ANA", "ThaiAirways"] + + +def test_unsupported_cache_expires(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """Past the TTL the note is ignored, so PP re-adding an airline (or a tier + change) heals without the user clearing anything.""" + cache = tmp_path / "unsupported.json" + cache.write_text(json.dumps({"ANA": time.time()})) + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_TTL_SECS", -1) + assert load_unsupported_airlines() == frozenset() + + +def test_unsupported_entries_expire_independently(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """Each entry ages on its own clock. + + Keying the TTL off the file mtime meant that learning ANY new airline + refreshed every existing entry. Since a run that learns one airline + rewrites the file, in steady state nothing ever expired and the cache + could not self-heal. + """ + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_TTL_SECS", 100) + now = time.time() + cache.write_text(json.dumps({"ANA": now - 500})) # already stale + remember_unsupported_airline("Finnair") # rewrites the file, mtime = now + assert load_unsupported_airlines() == frozenset({"Finnair"}) + + +def test_unsupported_cache_reads_legacy_list_format( + tmp_path: pathlib.Path, monkeypatch: Any +) -> None: + """The first version wrote a flat list. A freshly-written one is honoured, + so upgrading doesn't re-query every unsupported airline.""" + cache = tmp_path / "unsupported.json" + cache.write_text(json.dumps(["ANA", "Southwest"])) + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + assert load_unsupported_airlines() == frozenset({"ANA", "Southwest"}) + + +def test_legacy_list_entries_expire_by_file_mtime(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """Legacy entries carry no timestamp, so they are dated from the file's + mtime. Stamping them `now` on each read made them immortal: every read + refreshed them, so a legacy file could never age out.""" + cache = tmp_path / "unsupported.json" + cache.write_text(json.dumps(["ANA"])) + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_TTL_SECS", 100) + old = time.time() - 500 + os.utime(cache, (old, old)) + assert load_unsupported_airlines() == frozenset() + + +def test_remember_preserves_existing_timestamps(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """Adding an entry must not restamp its siblings.""" + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + original = time.time() - 42 + cache.write_text(json.dumps({"ANA": original})) + remember_unsupported_airline("Finnair") + written: Any = json.loads(cache.read_text()) + assert written["ANA"] == original + + +def test_unsupported_cache_tolerates_corrupt_file(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """Fails open — a bad cache costs a wasted request, never a hidden award.""" + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + cache.write_text("not json{") + assert load_unsupported_airlines() == frozenset() + cache.write_text("[1, 2, 3]") # right container, wrong element type + assert load_unsupported_airlines() == frozenset() + cache.write_text('["ANA", 42, null]') # legacy list, mixed types + assert load_unsupported_airlines() == frozenset({"ANA"}) + cache.write_text('{"ANA": "yesterday"}') # non-numeric timestamp + assert load_unsupported_airlines() == frozenset() + cache.write_text('{"ANA": true}') # bool is not a timestamp + assert load_unsupported_airlines() == frozenset() + + +def test_remember_survives_unwritable_cache(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """A cache-write failure is swallowed: the run continues and simply + re-queries that airline next time.""" + unwritable = tmp_path / "nodir" / "sub" / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", unwritable) + + def _boom(*_a: Any, **_kw: Any) -> None: + raise OSError("read-only fs") + + monkeypatch.setattr("flight_cli.pp.client.Path.mkdir", _boom) + remember_unsupported_airline("ANA") # must not raise + assert load_unsupported_airlines() == frozenset() + + +def test_expired_entry_is_restamped_on_a_fresh_rejection( + tmp_path: pathlib.Path, monkeypatch: Any +) -> None: + """An entry past its TTL is re-queried; when the server rejects it again + the note must be refreshed. Returning early on mere presence left it + permanently stale, so the airline was re-queried on every run.""" + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_TTL_SECS", 100) + stale = time.time() - 500 + cache.write_text(json.dumps({"ANA": stale})) + assert load_unsupported_airlines() == frozenset() # expired + remember_unsupported_airline("ANA") + assert load_unsupported_airlines() == frozenset({"ANA"}) # refreshed + written: Any = json.loads(cache.read_text()) + assert written["ANA"] > stale + + +def test_fresh_entry_is_not_restamped(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """A still-fresh entry keeps its original timestamp, so repeated + rejections can't extend it indefinitely past the TTL.""" + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_TTL_SECS", 100) + original = time.time() - 10 + cache.write_text(json.dumps({"ANA": original})) + remember_unsupported_airline("ANA") + written: Any = json.loads(cache.read_text()) + assert written["ANA"] == original diff --git a/tests/pp/test_client_request_retry.py b/tests/pp/test_client_request_retry.py index cf8e1dd..c01441a 100644 --- a/tests/pp/test_client_request_retry.py +++ b/tests/pp/test_client_request_retry.py @@ -8,20 +8,20 @@ from __future__ import annotations -from typing import TYPE_CHECKING +from typing import TYPE_CHECKING, Any import anyio import httpx +import pytest from flight_cli.pp import client as client_mod from flight_cli.pp.auth import Tokens -from flight_cli.pp.client import API_BASE, PPClient +from flight_cli.pp.client import API_BASE, PPApiError, PPClient if TYPE_CHECKING: + import pathlib from collections.abc import Callable - import pytest - def _tokens(access: str = "OLD_TOKEN") -> Tokens: return Tokens( @@ -159,3 +159,40 @@ async def go() -> httpx.Response: assert r.status_code == 401 assert len(refresh_calls) == 1, "refresh runs once even when retry also 401s" assert len(captured) == 2, "retry-once semantics: no third request" + + +# ───────────── catalog endpoints wrap HTTP failures in a domain error ───────────── + + +def test_pricing_info_raises_domain_error_not_httpx( + tmp_path: pathlib.Path, monkeypatch: Any +) -> None: + """AGENTS.md Principle 1: httpx.HTTPStatusError must not reach a caller.""" + monkeypatch.setattr("flight_cli.pp.client.PRICING_CACHE", tmp_path / "pricing.json") + handler, _ = _scripted_handler([httpx.Response(500, text="boom")]) + + async def go() -> None: + pp = _client_with_transport(_tokens(), httpx.MockTransport(handler)) + with pytest.raises(PPApiError) as ei: + _ = await pp.pricing_info() + assert ei.value.status == 500 + assert ei.value.endpoint == "/api/pricing-info" + assert isinstance(ei.value.__cause__, httpx.HTTPStatusError) + await pp.aclose() + + anyio.run(go) + + +def test_extension_config_raises_domain_error_not_httpx( + tmp_path: pathlib.Path, monkeypatch: Any +) -> None: + monkeypatch.setattr("flight_cli.pp.client.EXT_CONFIG_CACHE", tmp_path / "ext.json") + handler, _ = _scripted_handler([httpx.Response(503, text="down")]) + + async def go() -> None: + pp = _client_with_transport(_tokens(), httpx.MockTransport(handler)) + with pytest.raises(PPApiError): + _ = await pp.extension_config() + await pp.aclose() + + anyio.run(go) diff --git a/tests/pp/test_client_unsupported_airline.py b/tests/pp/test_client_unsupported_airline.py new file mode 100644 index 0000000..74954be --- /dev/null +++ b/tests/pp/test_client_unsupported_airline.py @@ -0,0 +1,145 @@ +# pyright: reportPrivateUsage=false +"""End-to-end wiring of the unsupported-airline negative cache. + +The helpers are unit-tested in test_client.py; these pin the behaviour that +actually saves the round-trips — that a 400 "unsupported airline" is recorded +rather than warned about, and that a recorded airline is never requested +again.""" + +from __future__ import annotations + +import json +from typing import TYPE_CHECKING, Any + +import anyio +import httpx + +from flight_cli.pp.auth import Tokens +from flight_cli.pp.client import API_BASE, PPClient, SearchSpec + +if TYPE_CHECKING: + import pathlib + + +def _tokens() -> Tokens: + return Tokens( + access_token="TOKEN", # noqa: S106 — dummy test value + refresh_token="REFRESH", # noqa: S106 — dummy test value + expires_at=9999999999, + user_email="test@example.com", + ) + + +def _client(transport: httpx.MockTransport) -> PPClient: + pp = PPClient(_tokens()) + pp._client = httpx.AsyncClient( + base_url=API_BASE, transport=transport, headers=pp._client.headers + ) + return pp + + +def _spec() -> SearchSpec: + return SearchSpec(origin="MSY", destination="MIA", date="2026-09-09") + + +_UNSUPPORTED = '{"error":"unsupported airline"}' + + +def test_unsupported_400_is_recorded(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + + def handler(_req: httpx.Request) -> httpx.Response: + return httpx.Response(400, text=_UNSUPPORTED) + + async def go() -> None: + pp = _client(httpx.MockTransport(handler)) + await pp.airline_search(_spec(), "ThaiAirways") + await pp.aclose() + + anyio.run(go) + written: Any = json.loads(cache.read_text()) + assert "ThaiAirways" in written + + +def test_recorded_airline_is_not_requested_again(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """The point of the cache: the skipped airline costs zero HTTP calls.""" + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + seen: list[str] = [] + + def handler(req: httpx.Request) -> httpx.Response: + body: Any = json.loads(req.content) + airline = str(body["airline"]) + seen.append(airline) + if airline == "ThaiAirways": + return httpx.Response(400, text=_UNSUPPORTED) + return httpx.Response(200, json={"outboundFlights": [], "inboundFlights": []}) + + async def go() -> None: + pp = _client(httpx.MockTransport(handler)) + await pp.airline_search_many(_spec(), ("American", "ThaiAirways")) + await pp.airline_search_many(_spec(), ("American", "ThaiAirways")) + await pp.aclose() + + anyio.run(go) + assert seen.count("ThaiAirways") == 1 # learned on run 1, skipped on run 2 + assert seen.count("American") == 2 # the working airline is unaffected + + +def test_transient_400_is_not_recorded(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """A 400 that isn't an unsupported-airline verdict must not blacklist a + working airline — it stays a warning and the airline is retried.""" + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + + def handler(_req: httpx.Request) -> httpx.Response: + return httpx.Response(400, text='{"error":"bad route"}') + + async def go() -> None: + pp = _client(httpx.MockTransport(handler)) + await pp.airline_search(_spec(), "American") + await pp.aclose() + + anyio.run(go) + assert not cache.exists() + + +def test_rate_limit_with_unsupported_body_is_not_recorded( + tmp_path: pathlib.Path, monkeypatch: Any +) -> None: + """Status is checked as well as body: a 429 must never blacklist.""" + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + + def handler(_req: httpx.Request) -> httpx.Response: + return httpx.Response(429, text=_UNSUPPORTED) + + async def go() -> None: + pp = _client(httpx.MockTransport(handler)) + await pp.airline_search(_spec(), "American") + await pp.aclose() + + anyio.run(go) + assert not cache.exists() + + +def test_concurrent_rejections_all_survive(tmp_path: pathlib.Path, monkeypatch: Any) -> None: + """airline_search_many fans out through a task group; every airline + rejected in one run must be recorded, not lost to read-modify-write + interleaving.""" + cache = tmp_path / "unsupported.json" + monkeypatch.setattr("flight_cli.pp.client.UNSUPPORTED_CACHE", cache) + rejected = ("ANA", "Finnair", "Southwest", "ThaiAirways", "CathayPacific") + + def handler(_req: httpx.Request) -> httpx.Response: + return httpx.Response(400, text=_UNSUPPORTED) + + async def go() -> None: + pp = _client(httpx.MockTransport(handler)) + await pp.airline_search_many(_spec(), rejected) + await pp.aclose() + + anyio.run(go) + written: Any = json.loads(cache.read_text()) + assert set(written) == set(rejected) diff --git a/tests/pp/test_gflight_adapter.py b/tests/pp/test_gflight_adapter.py index 7839a10..9367d83 100644 --- a/tests/pp/test_gflight_adapter.py +++ b/tests/pp/test_gflight_adapter.py @@ -278,3 +278,35 @@ def test_cash_hints_skip_slices_without_flight_id() -> None: ) hints = cash_hints_from_search_result(sr) assert hints == [] + + +# ───────── a price-less row must not take down the whole search ───────── + + +def test_price_string_tolerates_unsurfaced_price() -> None: + """fli types `FlightResult.price` as `NonNegativeFloat | None` — "None when + not surfaced", which Google does on some premium round-trip rows.""" + from flight_cli.pp.gflight_adapter import _price_string + + assert _price_string(_result(None, currency="USD")) == "" # pyright: ignore[reportArgumentType] + assert _price_string(_result(877.0)) == "USD877.00" + + +def test_one_priceless_row_does_not_discard_the_whole_response() -> None: + """The crash was `TypeError: unsupported format string passed to + NoneType.__format__`, raised while building the SearchResult — so ONE + price-less row destroyed every other itinerary in the response, not just + its own.""" + dep = datetime(2026, 8, 15, 18, 0) + arr = datetime(2026, 8, 16, 6, 0) + priceless = _result(None, _leg("100", "JFK", "LHR", dep, arr)) # pyright: ignore[reportArgumentType] + priced = _result(877.0, _leg("200", "JFK", "LHR", dep, arr)) + + sr = fli_results_to_search_result([priceless, priced]) # pyright: ignore[reportArgumentType] + + assert sr.solution_count == 2 + prices = [(s.ext.price if s.ext else None) for s in sr.solutions] + assert prices == ["", "USD877.00"] + # The cheapest-price notice ignores the unpriced row rather than crashing. + assert sr.currency_notice.ext is not None + assert sr.currency_notice.ext.price == "USD877.00" diff --git a/tests/pp/test_match.py b/tests/pp/test_match.py index 5eebf19..534e2a8 100644 --- a/tests/pp/test_match.py +++ b/tests/pp/test_match.py @@ -19,21 +19,24 @@ cash_match_key, cash_route_time_key, join, + same_metal, ) from flight_cli.providers.base import AwardFlight, CabinAward -def _itin(*slices_data: tuple[str, str, str, str]) -> Itinerary: +def _itin(*slices_data: tuple[str, ...]) -> Itinerary: """Build an Itinerary with the given slices. Each tuple is - (flight_number, departure_iso, origin_iata, destination_iata).""" + (flight_number, departure_iso, origin_iata, destination_iata) with an + optional 5th element for arrival_iso.""" slcs = [ Slice( - flights=[fn], - departure=dep, - origin=SliceEndpoint(code=o), - destination=SliceEndpoint(code=d), + flights=[t[0]], + departure=t[1], + origin=SliceEndpoint(code=t[2]), + destination=SliceEndpoint(code=t[3]), + arrival=(t[4] if len(t) > 4 else None), ) - for fn, dep, o, d in slices_data + for t in slices_data ] return Itinerary( displayTotal="USD500.00", @@ -65,25 +68,29 @@ def _award( miles: int = 47000, tax: float = 250.0, cabin: str = "Economy", - origin: str = "EWR", + origin: str = "JFK", dest: str = "LHR", funding_banks: list[str] | None = None, miles_to_cash_ratio: float = 0.0125, matched_id: str = "", + num_connections: int = 0, + arrival: str | None = None, + segment_flight_numbers: list[str] | None = None, ) -> AwardFlight: return AwardFlight( origin=origin, destination=dest, departure=dep, - arrival=dep, + arrival=arrival if arrival is not None else dep, flight_number=fn, - num_connections=0, + num_connections=num_connections, provider="PointsPath", program=program, miles_to_cash_ratio=miles_to_cash_ratio, funding_banks=funding_banks or ["Chase", "Bilt"], cabins=[CabinAward(cabin=cabin, miles=miles, tax_usd=tax, tax_currency="USD")], matched_google_flight_id=matched_id, + segment_flight_numbers=segment_flight_numbers or [], ) @@ -92,7 +99,7 @@ def _award( def test_cash_match_key_uppercases_and_strips_whitespace(): it = _itin(("ua 146", "2026-06-09T22:00:00", "JFK", "LHR")) - assert cash_match_key(it) == ("UA146", "2026-06-09") + assert cash_match_key(it) == ("UA146", "2026-06-09", "JFK", "LHR") def test_cash_match_key_empty_when_no_flights(): @@ -103,7 +110,7 @@ def test_cash_match_key_empty_when_no_flights(): def test_cash_match_key_handles_space_separated_iso(): """Some Matrix payloads return 'YYYY-MM-DD HH:MM' instead of ISO 'T'.""" it = _itin(("UA146", "2026-06-09 22:00", "JFK", "LHR")) - assert cash_match_key(it) == ("UA146", "2026-06-09") + assert cash_match_key(it) == ("UA146", "2026-06-09", "JFK", "LHR") def test_cash_match_key_uses_slice_index_for_return_leg(): @@ -111,8 +118,8 @@ def test_cash_match_key_uses_slice_index_for_return_leg(): ("UA146", "2026-06-09T22:00:00", "JFK", "LHR"), # outbound ("UA147", "2026-06-12T10:00:00", "LHR", "JFK"), # return ) - assert cash_match_key(it, slice_index=0) == ("UA146", "2026-06-09") - assert cash_match_key(it, slice_index=1) == ("UA147", "2026-06-12") + assert cash_match_key(it, slice_index=0) == ("UA146", "2026-06-09", "JFK", "LHR") + assert cash_match_key(it, slice_index=1) == ("UA147", "2026-06-12", "LHR", "JFK") def test_cash_match_key_out_of_range_slice_returns_none(): @@ -122,7 +129,7 @@ def test_cash_match_key_out_of_range_slice_returns_none(): def test_award_match_key_normalizes_consistently(): af = _award("ua 146", "2026-06-09T22:00:00") - assert award_match_key(af) == ("UA146", "2026-06-09") + assert award_match_key(af) == ("UA146", "2026-06-09", "JFK", "LHR") # ───────────────────────── route+time key ────────────────────────────────── @@ -244,25 +251,79 @@ def test_join_route_time_fallback_requires_minute_precision(): assert matches[0].awards == [] -def test_join_route_time_unions_with_flight_number_match(): - """If two providers' awards describe overlapping metal — one matches by - flight number, the other matches by route+time — both attach to the same - cash itinerary.""" +def test_join_arrival_time_resolves_codeshare_and_partner_in_one_bucket(): + """Arrival time is the real identity, so the codeshare bucket resolves + rather than failing closed. + + Cash AA6939 arrives 06:30. The BA174 award is the same physical aircraft + (same arrival) and attaches; an AS99 award sharing only the departure + minute is a different aircraft (different arrival) and does not. Keying + on route+departure alone could not tell these apart — on a live MSY + payload that key left 3 multi-carrier buckets of 41, and adding arrival + left 0 of 91. + """ + res = SearchResult( + solutions=[ + _itin(("AA6939", "2026-08-15T18:40:00", "JFK", "LHR", "2026-08-16T06:30:00")), + ] + ) + awards = [ + _award( + "BA174", + "2026-08-15T18:40:00", + program="American", + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:30:00", + ), + _award( + "AS99", + "2026-08-15T18:40:00", + program="Alaska", + miles=7500, + origin="JFK", + dest="LHR", + arrival="2026-08-16T07:55:00", + ), + ] + matches = join(res, awards) + assert [a.flight_number for a in matches[0].awards] == ["BA174"] + + +def test_join_falls_back_to_carrier_logic_when_arrival_missing(): + """Arrival is optional on both sides (`Slice.arrival` is nullable and a + provider may omit it), so the carrier resolution stays the backstop. With + no arrival anywhere, cash AA6939 + a BA174 and an AA6939 award is + unresolvable by time, and same-carrier-wins keeps only AA6939.""" res = SearchResult( solutions=[ _itin(("AA6939", "2026-08-15T18:40:00", "JFK", "LHR")), ] ) awards = [ - # Same metal under the OPERATING number (matches via route+time): _award("BA174", "2026-08-15T18:40:00", program="American", origin="JFK", dest="LHR"), - # Hypothetical second source reporting the cash marketing number directly - # (matches via primary key): _award("AA6939", "2026-08-15T18:40:00", program="VirginAtlantic", origin="JFK", dest="LHR"), ] matches = join(res, awards) - programs = {ao.program for ao in matches[0].awards} - assert programs == {"American", "VirginAtlantic"} + assert {a.program for a in matches[0].awards} == {"VirginAtlantic"} + + +def test_join_arrival_filter_does_not_wipe_bucket_when_awards_omit_arrival(): + """A cash arrival paired with awards that carry none must not zero the + bucket — the filter only applies when it actually matched something.""" + res = SearchResult( + solutions=[ + _itin(("AA6939", "2026-08-15T18:40:00", "JFK", "LHR", "2026-08-16T06:30:00")), + ] + ) + awards = [ + _award( + "BA174", "2026-08-15T18:40:00", program="American", origin="JFK", dest="LHR", arrival="" + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + assert matches[0].awards[0].flight_number == "BA174" # ────────────────────────────── join semantics ───────────────────────────── @@ -443,3 +504,1247 @@ def test_cash_without_flight_id_skips_matched_id_path(): matches = join(res, awards) # Joins via flight#+date heuristic, not matched-id. assert len(matches[0].awards) == 1 + + +# ────────────── carrier corroboration on the route+time fallback ────────────── +# +# Regression: a live MSY→MIA search attached a 9.1k Delta SkyMiles price to +# American's AA3539. Delta cannot ticket AA metal — no interline award +# agreement — and the two are genuinely different aircraft that happen to +# push back at the same minute. Route+time alone can't tell them apart. + + +def test_join_route_time_rejects_different_carrier_at_same_minute(): + """The bug, reduced. AA3539 and DL1424 both depart MSY→MIA at 10:45 on + 2026-09-09. Only the American award may attach.""" + res = SearchResult( + solutions=[ + _itin(("AA3539", "2026-09-09T10:45:00", "MSY", "MIA")), + ] + ) + awards = [ + _award( + "DL1424", + "2026-09-09T10:45:00", + program="Delta", + miles=9100, + origin="MSY", + dest="MIA", + ), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_join_route_time_keeps_same_carrier_at_same_minute(): + """The other half of the collision: the AA-programmed award on the same + route+minute still attaches.""" + res = SearchResult( + solutions=[ + _itin(("AA3539", "2026-09-09T10:45:00", "MSY", "MIA")), + ] + ) + awards = [ + _award( + "AA3539", + "2026-09-09T10:45:00", + program="American", + origin="MSY", + dest="MIA", + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + assert matches[0].awards[0].program == "American" + + +def test_join_route_time_partitions_a_real_collision(): + """Both flights present at once, as the live payload had them: each cash + itinerary keeps only its own carrier's award.""" + res = SearchResult( + solutions=[ + _itin(("AA3539", "2026-09-09T10:45:00", "MSY", "MIA")), + _itin(("DL1424", "2026-09-09T10:45:00", "MSY", "MIA")), + ] + ) + awards = [ + _award("DL1424", "2026-09-09T10:45:00", program="Delta", origin="MSY", dest="MIA"), + _award("AA3539", "2026-09-09T10:45:00", program="American", origin="MSY", dest="MIA"), + ] + matches = join(res, awards) + assert [a.program for a in matches[0].awards] == ["American"] + assert [a.program for a in matches[1].awards] == ["Delta"] + + +def test_join_route_time_still_bridges_oneworld_codeshare(): + """The fallback's reason for existing must survive the guard: AA-marketed + / BA-operated is a partner pair, so it still joins.""" + res = SearchResult( + solutions=[ + _itin(("AA6939", "2026-08-15T18:40:00", "JFK", "LHR")), + ] + ) + awards = [ + _award("BA174", "2026-08-15T18:40:00", program="American", origin="JFK", dest="LHR"), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + assert matches[0].awards[0].flight_number == "BA174" + + +def test_join_route_time_rejects_cross_alliance_pair(): + """United (Star) must not pick up an American (oneworld) award, even on + an identical route+minute.""" + res = SearchResult( + solutions=[ + _itin(("UA1122", "2026-08-15T18:40:00", "JFK", "LHR")), + ] + ) + awards = [ + _award("AA100", "2026-08-15T18:40:00", program="American", origin="JFK", dest="LHR"), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_join_route_time_allows_non_alliance_bilateral(): + """Delta/Virgin Atlantic codeshare across alliance lines via the JV.""" + res = SearchResult( + solutions=[ + _itin(("DL4321", "2026-08-15T18:40:00", "JFK", "LHR")), + ] + ) + awards = [ + _award("VS26", "2026-08-15T18:40:00", program="Delta", origin="JFK", dest="LHR"), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + + +def test_join_route_time_skipped_when_cash_slice_has_no_flight_number(): + """No cash flight number ⇒ no carrier to corroborate against ⇒ the + fallback can't fire. Fails closed: a wrong join invents an unbookable + price, a missed join just shows no award.""" + res = SearchResult( + solutions=[ + Itinerary( + displayTotal="USD500.00", + itinerary=ItineraryDetails( + slices=[ + Slice( + flights=[], + departure="2026-09-09T10:45:00", + origin=SliceEndpoint(code="MSY"), + destination=SliceEndpoint(code="MIA"), + ), + ], + carriers=[], + ), + ), + ] + ) + awards = [ + _award("AA3539", "2026-09-09T10:45:00", program="American", origin="MSY", dest="MIA"), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_join_flight_number_key_needs_no_carrier_corroboration(): + """The guard is scoped to the route+time fallback. An exact (flight#, + date) hit is self-corroborating and must be untouched — including when + the award's *program* differs from the operating carrier, which is the + normal partner-redemption case (Alaska miles on an AA flight).""" + res = SearchResult( + solutions=[ + _itin(("AA867", "2026-09-09T06:00:00", "MSY", "MIA")), + ] + ) + awards = [ + _award( + "AA867", + "2026-09-09T06:00:00", + program="Alaska", + miles=4500, + origin="MSY", + dest="MIA", + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + assert matches[0].awards[0].program == "Alaska" + + +def test_same_metal_helper_rejects_unparseable_carrier(): + assert same_metal("AA3539", "AA3539") is True + assert same_metal("AA3539", "DL1424") is False + assert same_metal("", "AA3539") is False + assert same_metal("AA", "AA3539") is False # too short to carry a number + + +# ───────── review round 2: bucket resolution, regionals, stop count ───────── +# +# The first fix gated the route+time fallback on `same_metal` and claimed the +# collision class was closed. It wasn't: same-alliance carriers compete on the +# same route at the same minute more often than cross-alliance ones do, so the +# guard relocated the bug instead of removing it — and made it less visible +# (an Alaska price on an AA row reads plausible; a Delta price did not). + + +def test_join_rejects_intra_alliance_collision_via_exact_carrier_preference(): + """The relocated bug. Cash AA118 JFK->LAX 10:45 with two awards in the + bucket: the real AA118 award and an AS17 award (different aircraft, same + minute, both oneworld). Only the American award may attach — otherwise + the renderer's lowest-miles pick shows 7.5k Alaska on the AA row.""" + res = SearchResult( + solutions=[ + _itin(("AA118", "2026-08-15T10:45:00", "JFK", "LAX")), + ] + ) + awards = [ + _award( + "AA118", + "2026-08-15T10:45:00", + program="American", + miles=25000, + origin="JFK", + dest="LAX", + ), + _award( + "AS17", "2026-08-15T10:45:00", program="Alaska", miles=7500, origin="JFK", dest="LAX" + ), + ] + matches = join(res, awards) + assert [a.program for a in matches[0].awards] == ["American"] + + +def test_join_rejects_ambiguous_multi_partner_bucket(): + """No exact-carrier award, and two DIFFERENT partner carriers share the + bucket. We cannot tell which is the cash flight's metal, so neither + attaches — guessing would be a coin flip on a price the user might book.""" + res = SearchResult( + solutions=[ + _itin(("AA118", "2026-08-15T10:45:00", "JFK", "LAX")), + ] + ) + awards = [ + _award("AS17", "2026-08-15T10:45:00", program="Alaska", origin="JFK", dest="LAX"), + _award("BA99", "2026-08-15T10:45:00", program="British Airways", origin="JFK", dest="LAX"), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_join_still_bridges_unambiguous_codeshare(): + """The fallback's reason for existing survives: a single partner carrier + alone in the bucket (no competing exact-carrier award) still joins.""" + res = SearchResult( + solutions=[ + _itin(("AA6939", "2026-08-15T18:40:00", "JFK", "LHR")), + ] + ) + awards = [ + _award("BA174", "2026-08-15T18:40:00", program="American", origin="JFK", dest="LHR"), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + assert matches[0].awards[0].flight_number == "BA174" + + +def test_join_bridges_mainline_marketed_regional_operated(): + """The dominant real codeshare shape, and the repo's own documented + example (docs/memories/gf_routing_and_carriers.md): LH9498 marketed, + EN8858 (Air Dolomiti) operated. An alliance-only table rejected this — + a feeder is not an alliance member.""" + res = SearchResult( + solutions=[ + _itin(("LH9498", "2026-08-15T09:15:00", "FRA", "FLR")), + ] + ) + awards = [ + _award("EN8858", "2026-08-15T09:15:00", program="United", origin="FRA", dest="FLR"), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + assert matches[0].awards[0].flight_number == "EN8858" + + +def test_regional_operator_mapping_is_directional_not_symmetric(): + """MQ and OH are both American regionals but have no relationship with + each other. A symmetric table would wrongly pair them.""" + assert same_metal("AA3539", "MQ3539") is True # mainline -> its regional + assert same_metal("MQ3539", "OH1234") is False # two regionals of the same mainline + assert same_metal("MQ3539", "DL1424") is False # regional -> unrelated mainline + + +def test_join_rejects_connecting_award_on_nonstop_cash_row(): + """A 1-stop award is cheaper than the nonstop it would render beside, so + it wins the lowest-miles pick — and the row still prints 'nonstop', + because stops come from the cash slice. Connection count must agree.""" + res = SearchResult( + solutions=[ + _itin(("AA100", "2026-08-15T18:30:00", "JFK", "LHR")), + ] + ) + awards = [ + _award( + "AA100", + "2026-08-15T18:30:00", + program="American", + miles=12000, + origin="JFK", + dest="LHR", + num_connections=1, + ), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_join_accepts_award_whose_stop_count_agrees(): + """The other side of the stop-count check: a genuine nonstop award on a + nonstop cash row still attaches.""" + res = SearchResult( + solutions=[ + _itin(("AA100", "2026-08-15T18:30:00", "JFK", "LHR")), + ] + ) + awards = [ + _award( + "AA100", + "2026-08-15T18:30:00", + program="American", + origin="JFK", + dest="LHR", + num_connections=0, + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + + +def test_join_flight_number_key_requires_matching_route(): + """Flight numbers repeat across a carrier's daily rotations, so + (flight#, date) alone is not an identity: an AA100 JFK->LHR cash row + used to attach an AA100 MIA->DFW award.""" + res = SearchResult( + solutions=[ + _itin(("AA100", "2026-08-15T18:00:00", "JFK", "LHR")), + ] + ) + awards = [ + _award( + "AA100", "2026-08-15T06:30:00", program="American", miles=7500, origin="MIA", dest="DFW" + ), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_partner_groups_have_no_unintended_transitivity(): + """Membership is tested per-group, so overlapping groups (DL is in both + SkyTeam and the DL/VS bilateral) must not chain: VS must not reach AF.""" + assert same_metal("DL1", "VS2") is True # direct bilateral + assert same_metal("VS1", "AF2") is False # would require chaining through DL + assert same_metal("B61", "AA2") is False # via AS: B6-AS bilateral, AS-AA oneworld + assert same_metal("EK1", "AA2") is False # via QF: EK-QF bilateral, QF-AA oneworld + + +def test_partner_group_codes_are_well_formed(): + """Structural pin: a malformed entry would silently never match, since + `_carrier` only ever produces two-character prefixes.""" + # DIVERGE: reportPrivateUsage — these are module-internal reference + # tables, not API. A structural test is exactly the case where reaching + # in is correct; exporting them publicly to satisfy the rule would widen + # the surface for a test's benefit. + from flight_cli.pp.match import ( + _PARTNER_GROUPS, # pyright: ignore[reportPrivateUsage] + _REGIONAL_OPERATORS, # pyright: ignore[reportPrivateUsage] + ) + + for group in _PARTNER_GROUPS: + assert len(group) >= 2, f"degenerate group: {group}" + for code in group: + assert len(code) == 2, f"not a 2-char IATA code: {code!r}" + for mainline, regionals in _REGIONAL_OPERATORS.items(): + assert len(mainline) == 2, f"not a 2-char IATA code: {mainline!r}" + for code in regionals: + assert len(code) == 2, f"not a 2-char IATA code: {code!r}" + assert code != mainline, f"{mainline} lists itself as its own regional" + + +def test_same_metal_rejects_award_side_empty_flight_number(): + """Fails closed on an unparseable award-side carrier, mirroring the + cash-side case. A missed match shows no award; a wrong one invents a + price the user might try to book.""" + assert same_metal("AA3539", "") is False + assert same_metal("AA3539", None) is False + + +# ─────────── multi-stop slices + wire-format timestamps (review r2) ─────────── + + +def _itin_multi( + flights: list[str], + dep: str, + o: str, + d: str, + stops: list[str], + arrival: str | None = None, +) -> Itinerary: + """A connecting itinerary: N flights, N-1 intermediate stops.""" + s = Slice( + flights=flights, + departure=dep, + arrival=arrival, + origin=SliceEndpoint(code=o), + destination=SliceEndpoint(code=d), + stops=[SliceEndpoint(code=c) for c in stops], + ) + return Itinerary( + displayTotal="USD500.00", + itinerary=ItineraryDetails(slices=[s], carriers=[]), + ) + + +def test_join_matches_one_stop_award_to_one_stop_cash(): + """The stop-count check is an equality, not a nonstop-only filter: a + connecting cash slice must still match its own connecting award.""" + res = SearchResult( + solutions=[ + _itin_multi(["DL2542", "DL719"], "2026-09-09T13:00:00", "MSY", "MIA", ["ATL"]), + ] + ) + awards = [ + _award( + "DL2542", + "2026-09-09T13:00:00", + program="Delta", + origin="MSY", + dest="MIA", + num_connections=1, + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + + +def test_join_rejects_nonstop_award_on_connecting_cash_row(): + """The other direction: a nonstop award is a different (better) product + than the 1-stop cash itinerary it would render against.""" + res = SearchResult( + solutions=[ + _itin_multi(["DL2542", "DL719"], "2026-09-09T13:00:00", "MSY", "MIA", ["ATL"]), + ] + ) + awards = [ + _award( + "DL2542", + "2026-09-09T13:00:00", + program="Delta", + origin="MSY", + dest="MIA", + num_connections=0, + ), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_slice_stop_count_falls_back_to_segment_count(): + """`stops` is authoritative when Matrix populates it; the gflight adapter + may not, so a 2-segment slice with no `stops` still counts as 1.""" + from flight_cli.pp.match import _slice_stop_count # pyright: ignore[reportPrivateUsage] + + populated = Slice(flights=["DL1", "DL2"], stops=[SliceEndpoint(code="ATL")]) + assert _slice_stop_count(populated) == 1 + derived = Slice(flights=["DL1", "DL2"]) # no stops[] + assert _slice_stop_count(derived) == 1 + nonstop = Slice(flights=["DL1"]) + assert _slice_stop_count(nonstop) == 0 + + +def test_arrival_match_survives_matrix_utc_offset_wire_format(): + """Matrix emits offset-aware local times ('2026-09-09T09:04-04:00') whose + offset can even differ from the departure's ('...T06:00-05:00' for the same + flight), while PointsPath emits naive local ('2026-09-09T09:04:00'). Both + are local-at-the-airport and `_iso_minute` truncates before the offset, so + they compare equal — this pins that, since a format drift here would + silently turn the arrival discriminator into a no-op.""" + res = SearchResult( + solutions=[ + _itin(("AA867", "2026-09-09T06:00-05:00", "MSY", "MIA", "2026-09-09T09:04-04:00")), + ] + ) + awards = [ + _award( + "AA867", + "2026-09-09T06:00:00", + program="American", + origin="MSY", + dest="MIA", + arrival="2026-09-09T09:04:00", + ), + # Same route+departure, different aircraft (later arrival) — must not win. + _award( + "AA999", + "2026-09-09T06:00:00", + program="Alaska", + miles=1000, + origin="MSY", + dest="MIA", + arrival="2026-09-09T10:30:00", + ), + ] + matches = join(res, awards) + assert [a.flight_number for a in matches[0].awards] == ["AA867"] + + +def test_cash_first_flight_number_direct(): + """Direct coverage of the guard branches, matching its siblings.""" + from flight_cli.pp.match import cash_first_flight_number + + it = _itin(("ua 146", "2026-06-09T22:00:00", "JFK", "LHR")) + assert cash_first_flight_number(it) == "UA146" + assert cash_first_flight_number(it, slice_index=5) == "" + no_flights = Itinerary(itinerary=ItineraryDetails(slices=[Slice(flights=[])])) + assert cash_first_flight_number(no_flights) == "" + + +def test_match_keys_require_complete_route(): + """A slice or award missing origin/destination yields no key rather than a + partial one that could collide with an unrelated flight.""" + partial = Itinerary( + itinerary=ItineraryDetails( + slices=[ + Slice( + flights=["AA100"], + departure="2026-08-15T18:00:00", + origin=SliceEndpoint(code="JFK"), + destination=None, + ), + ], + ), + ) + assert cash_match_key(partial) is None + assert award_match_key(_award("AA100", "2026-08-15T18:00:00", origin="", dest="LHR")) is None + assert award_match_key(_award("AA100", "", origin="JFK", dest="LHR")) is None + + +# ───────── arrival must narrow WITHIN carrier stages, not ahead of them ───────── +# +# The arrival discriminator was first applied to the whole bucket before the +# carrier rules ran. That let a wrong-carrier award whose arrival happened to +# match survive while the CORRECT same-carrier award — one that merely omitted +# its arrival — was deleted before the exact-carrier rule ever saw it, putting +# another aircraft's price on the row. Ordering here is load-bearing. + + +def test_arrival_filter_does_not_delete_correct_metal_in_mixed_bucket(): + """The P0. Cash AA6939 arrives 06:30. BA174 is the right metal but omits + its arrival; AS99 is different metal that happens to arrive 06:30. + Filtering by arrival first left only AS99 and rendered 7.5k Alaska on the + American row.""" + res = SearchResult( + solutions=[ + _itin(("AA6939", "2026-08-15T18:40:00", "JFK", "LHR", "2026-08-16T06:30:00")), + ] + ) + awards = [ + _award( + "BA174", + "2026-08-15T18:40:00", + program="American", + miles=60000, + origin="JFK", + dest="LHR", + arrival="", + ), + _award( + "AS99", + "2026-08-15T18:40:00", + program="Alaska", + miles=7500, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:30:00", + ), + ] + matches = join(res, awards) + assert [a.program for a in matches[0].awards] != ["Alaska"] + # Two distinct partner carriers with incomplete arrival data is genuinely + # unresolvable, so the safe answer is no award at all. + assert matches[0].awards == [] + + +def test_arrival_filter_abstains_on_partial_coverage_keeping_exact_carrier(): + """Same shape, but the same-carrier award is the one missing an arrival. + It must still win — partial arrival coverage is not evidence.""" + res = SearchResult( + solutions=[ + _itin(("AA118", "2026-08-15T10:45:00", "JFK", "LAX", "2026-08-15T14:05:00")), + ] + ) + awards = [ + _award( + "AA118", + "2026-08-15T10:45:00", + program="American", + miles=25000, + origin="JFK", + dest="LAX", + arrival="", + ), + _award( + "AS17", + "2026-08-15T10:45:00", + program="Alaska", + miles=7500, + origin="JFK", + dest="LAX", + arrival="2026-08-15T14:05:00", + ), + ] + matches = join(res, awards) + assert [a.program for a in matches[0].awards] == ["American"] + + +def test_arrival_separates_same_carrier_rotations(): + """What arrival is actually for: two AA flights the carrier rules cannot + tell apart, sharing a departure minute. Only the matching arrival wins.""" + res = SearchResult( + solutions=[ + _itin(("AA867", "2026-09-09T06:00-05:00", "MSY", "MIA", "2026-09-09T09:04-04:00")), + ] + ) + awards = [ + _award( + "AA867", + "2026-09-09T06:00:00", + program="American", + miles=25000, + origin="MSY", + dest="MIA", + arrival="2026-09-09T09:04:00", + ), + _award( + "AA999", + "2026-09-09T06:00:00", + program="American", + miles=9000, + origin="MSY", + dest="MIA", + arrival="2026-09-09T10:30:00", + ), + ] + matches = join(res, awards) + assert [a.flight_number for a in matches[0].awards] == ["AA867"] + + +def test_arrival_disagreement_drops_the_award_even_if_it_is_the_only_one(): + """Disagreeing arrival is evidence of different metal, and we act on it + even when that leaves the row with no award. + + Real schedule sources do drift: a cross-check of the seats.aero fixture + against the live Matrix cache found AA106 differing by 5 minutes (19:20 vs + 19:15). So this WILL cost some legitimate matches. That trade is deliberate + — a dropped award shows an empty cell the user can investigate, while a + re-admitted one prints a confident price for another aircraft. Absence of + evidence (no arrival at all) is still admitted; only contradiction is not. + + If drift turns out to be common in practice, the fix is a tolerance window + here, not restoring a wholesale fallback. + """ + res = SearchResult( + solutions=[ + _itin(("AA106", "2026-08-15T19:20:00", "JFK", "LHR", "2026-08-16T07:30:00")), + ] + ) + awards = [ + _award( + "AA106", + "2026-08-15T19:20:00", + program="American", + origin="JFK", + dest="LHR", + arrival="2026-08-16T07:25:00", + ), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_arrival_missing_on_award_is_still_admitted(): + """Absence of evidence is not evidence: an award that omits its arrival + stays eligible and is resolved by the carrier rules.""" + res = SearchResult( + solutions=[ + _itin(("AA106", "2026-08-15T19:20:00", "JFK", "LHR", "2026-08-16T07:30:00")), + ] + ) + awards = [ + _award( + "AA106", "2026-08-15T19:20:00", program="American", origin="JFK", dest="LHR", arrival="" + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + + +def test_primary_key_resolves_sibling_journeys_by_arrival(): + """The second P0. (flight#, date, origin, dest) keys on the FIRST segment, + so every connecting journey starting on that flight collapses onto one + key. The repo's seats.aero fixture holds three distinct AA1444 JFK->LHR + journeys (arriving 06:55, 09:05, 12:50) — unresolved, the renderer's + lowest-miles pick quotes the 12:50 journey's fare on the 06:55 row.""" + res = SearchResult( + solutions=[ + _itin_multi( + ["AA1444", "AA200"], + "2026-08-15T18:00:00", + "JFK", + "LHR", + ["BOS"], + arrival="2026-08-16T06:55:00", + ), + ] + ) + awards = [ + _award( + "AA1444", + "2026-08-15T18:00:00", + program="American Airlines", + miles=115500, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:55:00", + num_connections=1, + ), + _award( + "AA1444", + "2026-08-15T18:00:00", + program="American Airlines", + miles=104000, + origin="JFK", + dest="LHR", + arrival="2026-08-16T12:50:00", + num_connections=1, + ), + ] + matches = join(res, awards) + assert [a.cabins[0].miles for a in matches[0].awards] == [115500] + + +def test_matched_id_path_requires_carrier_corroboration(): + """The third P0. PP mints matchedGoogleFlightId from a hint we supply and + its matcher is documented as loose, so the echoed ID is a claim, not proof. + A Delta award must not ride it onto an American row.""" + res = SearchResult( + solutions=[ + _itin_with_id("AA3539", "2026-09-09T10:45:00", "MSY", "MIA", flight_id="XyZ123"), + ] + ) + awards = [ + _award( + "DL1424", + "2026-09-09T10:45:00", + program="Delta", + miles=9100, + origin="MSY", + dest="MIA", + matched_id="XyZ123", + ), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_matched_id_path_still_bridges_a_real_codeshare(): + """...but the path keeps working for the codeshare it exists to serve.""" + res = SearchResult( + solutions=[ + _itin_with_id("AA6939", "2026-08-15T18:40:00", "JFK", "LHR", flight_id="XyZ123"), + ] + ) + awards = [ + _award( + "BA174", + "2026-08-15T18:40:00", + program="American", + origin="JFK", + dest="LHR", + matched_id="XyZ123", + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + + +# ─────────── codex adversarial pass: buckets on the other two keys ─────────── + + +def test_matched_id_bucket_is_resolved_not_just_filtered_on_return_leg(): + """Codex P0. Several awards can share one matchedGoogleFlightId. Filtering + that bucket with `same_metal` alone still admits a partner beside the true + same-carrier award, and the renderer's lowest-miles pick then shows the + partner's 7.5k Alaska price on an American row. Return leg, because + slice_index=1 had almost no coverage.""" + it = Itinerary( + displayTotal="USD500.00", + itinerary=ItineraryDetails( + slices=[ + Slice( + flights=["AA100"], + departure="2026-08-15T18:00:00", + origin=SliceEndpoint(code="JFK"), + destination=SliceEndpoint(code="LAX"), + ), + Slice( + flights=["AA117"], + departure="2026-08-20T10:00:00", + origin=SliceEndpoint(code="LAX"), + destination=SliceEndpoint(code="JFK"), + flight_id="RET1", + ), + ], + carriers=[], + ), + ) + awards = [ + _award( + "AA117", + "2026-08-20T10:00:00", + program="American", + miles=25000, + origin="LAX", + dest="JFK", + matched_id="RET1", + ), + _award( + "AS19", + "2026-08-20T10:00:00", + program="Alaska", + miles=7500, + origin="LAX", + dest="JFK", + matched_id="RET1", + ), + ] + matches = join(SearchResult(solutions=[it]), awards, slice_index=1) + assert [a.program for a in matches[0].awards] == ["American"] + + +def test_segment_disagreement_separates_journeys_sharing_first_flight(): + """Codex P0. seats.aero returns both "AA1444, BA216" and "AA1444, AA100" + on one JFK->LHR date. Every key here identifies a journey by segment 0, so + without the full segment list they collapse and the cheaper journey's fare + prints on the other's row.""" + res = SearchResult( + solutions=[ + _itin_multi( + ["AA1444", "BA216"], + "2026-08-15T18:00:00", + "JFK", + "LHR", + ["BOS"], + arrival="2026-08-16T06:55:00", + ), + ] + ) + awards = [ + _award( + "AA1444", + "2026-08-15T18:00:00", + program="American Airlines", + miles=60000, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:55:00", + num_connections=1, + segment_flight_numbers=["AA1444", "BA216"], + ), + _award( + "AA1444", + "2026-08-15T18:00:00", + program="American Airlines", + miles=12000, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:55:00", + num_connections=1, + segment_flight_numbers=["AA1444", "AA100"], + ), + ] + matches = join(res, awards) + assert [a.cabins[0].miles for a in matches[0].awards] == [60000] + + +def test_segment_check_admits_providers_that_omit_segments(): + """PointsPath sends only the first flight number. An empty segment list is + absence of evidence, so it must not be read as disagreement.""" + res = SearchResult( + solutions=[ + _itin_multi( + ["AA1444", "BA216"], + "2026-08-15T18:00:00", + "JFK", + "LHR", + ["BOS"], + arrival="2026-08-16T06:55:00", + ), + ] + ) + awards = [ + _award( + "AA1444", + "2026-08-15T18:00:00", + program="American", + miles=60000, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:55:00", + num_connections=1, + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + + +def test_exact_arrival_match_beats_cheaper_sibling_with_no_arrival(): + """Codex P0. A no-arrival sibling used to sit beside the award that + positively confirmed the cash flight, and its lower price won the cell — + silently preferring the unverified candidate over the verified one.""" + res = SearchResult( + solutions=[ + _itin_multi( + ["AA1444", "BA216"], + "2026-08-15T18:00:00", + "JFK", + "LHR", + ["BOS"], + arrival="2026-08-16T06:55:00", + ), + ] + ) + awards = [ + _award( + "AA1444", + "2026-08-15T18:00:00", + program="American Airlines", + miles=60000, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:55:00", + num_connections=1, + ), + _award( + "AA1444", + "2026-08-15T18:00:00", + program="American Airlines", + miles=10000, + origin="JFK", + dest="LHR", + arrival="", + num_connections=1, + ), + ] + matches = join(res, awards) + assert [a.cabins[0].miles for a in matches[0].awards] == [60000] + + +def test_arrival_never_picks_between_carriers(): + """The inverse trap. Arrival separates two rotations of the SAME carrier; + it must not choose between carriers, or a partner that merely omits its + arrival loses to an unrelated carrier that happens to publish a matching + one. Cash AA6939 + BA174 (no arrival) + AS99 (matching arrival) is + unresolvable, not an Alaska match.""" + res = SearchResult( + solutions=[ + _itin(("AA6939", "2026-08-15T18:40:00", "JFK", "LHR", "2026-08-16T06:30:00")), + ] + ) + awards = [ + _award( + "BA174", + "2026-08-15T18:40:00", + program="American", + miles=60000, + origin="JFK", + dest="LHR", + arrival="", + ), + _award( + "AS99", + "2026-08-15T18:40:00", + program="Alaska", + miles=7500, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:30:00", + ), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +# ───────── codex correctness pass: single resolution over the union ───────── + + +def test_one_bucket_cannot_readmit_what_another_rejected(): + """Codex P0, and the architectural one. + + Resolving each key's bucket separately and unioning the winners lets a + bucket that sees only part of the field decide on partial information. + Cash AA6939 with a BA174 award (found by route+time) and an AS99 award + (found by matched-ID): route+time sees two different partner carriers and + correctly abstains, but the matched-ID bucket sees AS99 alone, calls it an + unambiguous single partner, and admits it — rendering 7.5k Alaska on an + American row. Keys are discovery; resolution is one judgment over + everything they found. + """ + it = _itin_with_id("AA6939", "2026-08-15T18:40:00", "JFK", "LHR", flight_id="F1") + awards = [ + _award( + "BA174", + "2026-08-15T18:40:00", + program="American", + miles=60000, + origin="JFK", + dest="LHR", + ), + _award( + "AS99", + "2026-08-15T18:40:00", + program="Alaska", + miles=7500, + origin="JFK", + dest="LHR", + matched_id="F1", + ), + ] + matches = join(SearchResult(solutions=[it]), awards) + assert matches[0].awards == [] + + +def test_same_carrier_different_number_needs_arrival_proof(): + """Codex P0. An airline does not sell one departure under two of its own + numbers, so AA999 is not cash AA867 — only a genuine codeshare (different + carrier) explains a number mismatch. Without a matching arrival it must + fail closed, not ride in on the missing-arrival allowance.""" + res = SearchResult( + solutions=[ + _itin(("AA867", "2026-09-09T06:00:00", "MSY", "MIA", "2026-09-09T09:04:00")), + ] + ) + awards = [ + _award( + "AA999", + "2026-09-09T06:00:00", + program="American", + miles=9000, + origin="MSY", + dest="MIA", + arrival="", + ), + ] + matches = join(res, awards) + assert matches[0].awards == [] + + +def test_same_carrier_different_number_accepted_with_matching_arrival(): + """The other side: a renumbered award for the same departure is accepted + once its arrival positively confirms the flight.""" + res = SearchResult( + solutions=[ + _itin(("AA867", "2026-09-09T06:00:00", "MSY", "MIA", "2026-09-09T09:04:00")), + ] + ) + awards = [ + _award( + "AA999", + "2026-09-09T06:00:00", + program="American", + miles=9000, + origin="MSY", + dest="MIA", + arrival="2026-09-09T09:04:00", + ), + ] + matches = join(res, awards) + assert [a.flight_number for a in matches[0].awards] == ["AA999"] + + +def test_wrong_stop_count_candidate_cannot_suppress_a_valid_codeshare(): + """Codex P2. Connection count is an objective property, so mismatches are + dropped BEFORE resolution. Filtering them afterwards let an ineligible + candidate make the field look ambiguous and suppress a codeshare that + would have won on its own.""" + res = SearchResult( + solutions=[ + _itin(("AA6939", "2026-08-15T18:40:00", "JFK", "LHR", "2026-08-16T06:30:00")), + ] + ) + awards = [ + # Valid nonstop codeshare — should win. + _award( + "BA174", + "2026-08-15T18:40:00", + program="American", + miles=60000, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:30:00", + ), + # Ineligible: 1 stop against a nonstop cash slice. A second carrier, + # so pre-filtering is what stops it looking like carrier ambiguity. + _award( + "AS99", + "2026-08-15T18:40:00", + program="Alaska", + miles=7500, + origin="JFK", + dest="LHR", + arrival="2026-08-16T06:30:00", + num_connections=1, + ), + ] + matches = join(res, awards) + assert [a.flight_number for a in matches[0].awards] == ["BA174"] + + +# ───────── journey shape: connection airports (the cross-provider signal) ───────── + + +def _award_stops( + fn: str, + dep: str, + arr: str, + miles: int, + stops: list[str], + *, + conns: int = 1, + segs: list[str] | None = None, +) -> AwardFlight: + return AwardFlight( + origin="MSY", + destination="LHR", + departure=dep, + arrival=arr, + flight_number=fn, + num_connections=conns, + provider="PointsPath", + program="American", + miles_to_cash_ratio=0.0125, + funding_banks=["Chase"], + stop_airports=stops, + segment_flight_numbers=segs or [], + cabins=[CabinAward(cabin="Economy", miles=miles, tax_usd=5.6, tax_currency="USD")], + ) + + +def _cash_conn(flights: list[str], dep: str, stops: list[str], arr: str) -> Itinerary: + return Itinerary( + displayTotal="USD900.00", + itinerary=ItineraryDetails( + slices=[ + Slice( + flights=flights, + departure=dep, + arrival=arr, + origin=SliceEndpoint(code="MSY"), + destination=SliceEndpoint(code="LHR"), + stops=[SliceEndpoint(code=c) for c in stops], + ), + ], + carriers=[], + ), + ) + + +def test_connection_airport_separates_pointspath_journeys(): + """PointsPath sends no segment list, so segment numbers can't separate its + journeys — but it does send `stops`, and Matrix fills the comparable + `Slice.stops`. Live MSY->LHR has four AA1650 journeys sharing a 12:22 + departure and one connection, differing only in hub and arrival. + """ + res = SearchResult( + solutions=[ + _cash_conn(["AA1650", "AA100"], "2026-09-09T12:22:00", ["DFW"], "2026-09-10T06:20:00") + ] + ) + awards = [ + _award_stops("AA1650", "2026-09-09T12:22:00", "2026-09-10T06:20:00", 60000, ["DFW"]), + # Cheaper, so it would win the renderer's pick — but it routes via ORD. + _award_stops("AA1650", "2026-09-09T12:22:00", "2026-09-10T06:20:00", 9000, ["ORD"]), + ] + matches = join(res, awards) + assert [a.cabins[0].miles for a in matches[0].awards] == [60000] + + +def test_missing_stop_airports_is_not_disagreement(): + """A provider that omits connection airports must still match — absence of + evidence is not contradiction.""" + res = SearchResult( + solutions=[ + _cash_conn(["AA1650", "AA100"], "2026-09-09T12:22:00", ["DFW"], "2026-09-10T06:20:00") + ] + ) + awards = [ + _award_stops("AA1650", "2026-09-09T12:22:00", "2026-09-10T06:20:00", 60000, []), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 + + +def test_both_shape_signals_apply_together(): + """Connection airports and segment numbers are independent checks; either + one contradicting is enough to drop a candidate.""" + res = SearchResult( + solutions=[ + _cash_conn(["AA1650", "AA100"], "2026-09-09T12:22:00", ["DFW"], "2026-09-10T06:20:00") + ] + ) + awards = [ + # Right hub, wrong second segment. + _award_stops( + "AA1650", + "2026-09-09T12:22:00", + "2026-09-10T06:20:00", + 9000, + ["DFW"], + segs=["AA1650", "AA999"], + ), + _award_stops( + "AA1650", + "2026-09-09T12:22:00", + "2026-09-10T06:20:00", + 60000, + ["DFW"], + segs=["AA1650", "AA100"], + ), + ] + matches = join(res, awards) + assert [a.cabins[0].miles for a in matches[0].awards] == [60000] + + +def test_nonstop_cash_row_is_unaffected_by_shape_checks(): + """A nonstop has nothing past segment 0 to contradict, so the shape checks + must not filter anything.""" + res = SearchResult( + solutions=[ + _itin(("AA867", "2026-09-09T06:00:00", "MSY", "MIA", "2026-09-09T09:04:00")), + ] + ) + awards = [ + _award( + "AA867", + "2026-09-09T06:00:00", + program="American", + miles=25000, + origin="MSY", + dest="MIA", + arrival="2026-09-09T09:04:00", + ), + ] + matches = join(res, awards) + assert len(matches[0].awards) == 1 diff --git a/tests/seats_aero/test_models.py b/tests/seats_aero/test_models.py index a0414e7..312c1a3 100644 --- a/tests/seats_aero/test_models.py +++ b/tests/seats_aero/test_models.py @@ -67,7 +67,9 @@ def test_with_trips_trip_fields() -> None: t = trips[0] # FlightNumbers is a comma-joined string in segment order. assert "," in t.FlightNumbers or "-" not in t.FlightNumbers - # DepartsAt is ISO 8601 UTC. + # DepartsAt arrives with a 'Z' suffix — which upstream mislabels: the + # value is local time at the airport. Pinned as the raw wire shape; the + # provider strips it (see test_seats_aero_timestamps_are_normalized_local). assert t.DepartsAt.endswith("Z") or "+" in t.DepartsAt # Cabin is lowercased. assert t.Cabin in {"economy", "premium", "business", "first"} diff --git a/tests/seats_aero/test_provider.py b/tests/seats_aero/test_provider.py index 9e4a160..8cc0c31 100644 --- a/tests/seats_aero/test_provider.py +++ b/tests/seats_aero/test_provider.py @@ -128,3 +128,39 @@ def test_group_converts_tax_cents_to_usd() -> None: def test_group_empty_input_returns_empty() -> None: assert _group_trips_to_awards([], tax_currency="USD") == [] + + +def test_seats_aero_timestamps_are_normalized_to_naive_local() -> None: + """seats.aero labels DepartsAt/ArrivesAt as UTC with a 'Z', but the values + are local time at each airport — honouring the Z yields ~12.1h JFK->LHR + nonstops against a real ~7h. The provider strips the suffix so + AwardFlight timestamps mean naive-local for every provider, matching what + Matrix and PointsPath supply and what the matcher compares. + """ + from flight_cli.providers.seats_aero.provider import _local_naive + + assert _local_naive("2026-08-15T06:30:00Z") == "2026-08-15T06:30:00" + assert _local_naive("2026-08-15T06:30:00") == "2026-08-15T06:30:00" + assert _local_naive("") == "" + + +def test_cabin_labels_match_the_cli_canonical_vocabulary() -> None: + """The renderer selects awards by exact cabin-string equality, so a + provider label that differs from the CLI's canonical name makes those + awards vanish from the column. seats.aero's "Premium" never equalled + "Premium economy".""" + from flight_cli.pp.cli import _normalize_cabin + from flight_cli.providers.seats_aero.provider import _cabin_label + + for slug in ("economy", "premium", "business", "first"): + assert _cabin_label(slug) == _normalize_cabin(slug) + + +def test_cabin_slug_round_trips_for_the_outbound_filter() -> None: + """The API filter needs seats.aero's own slug. A bare `.lower()` sent + "premium economy" once the canonical label gained a space, and matched + nothing.""" + from flight_cli.providers.seats_aero.provider import _cabin_label, _cabin_slug + + for slug in ("economy", "premium", "business", "first"): + assert _cabin_slug(_cabin_label(slug)) == slug diff --git a/tests/test_cli_pax_and_pick.py b/tests/test_cli_pax_and_pick.py new file mode 100644 index 0000000..2fc4da9 --- /dev/null +++ b/tests/test_cli_pax_and_pick.py @@ -0,0 +1,192 @@ +# pyright: reportPrivateUsage=false +"""Regressions for cli.py findings from the codex adversarial pass. + +All four were the house failure class: a plausible artifact making a claim +that isn't true of the search the user asked for. +""" + +from __future__ import annotations + +from datetime import date +from typing import Any + +from flight_cli.cli import _pinned_solution_index, _seated_pax +from flight_cli.domain import Leg, Pax, SearchOptions, SpecificDateSearch +from flight_cli.models import ( + Itinerary, + ItineraryDetails, + SearchResult, + Slice, + SliceEndpoint, +) + + +def _res(n: int) -> SearchResult: + sols = [ + Itinerary( + id=f"sol-{i}", + displayTotal=f"USD{i:03d}.00", + itinerary=ItineraryDetails( + slices=[ + Slice( + flights=[f"AA{i}"], + departure="2026-09-01T06:00", + origin=SliceEndpoint(code="JFK"), + destination=SliceEndpoint(code="LHR"), + ), + ], + carriers=[], + ), + ) + for i in range(1, n + 1) + ] + return SearchResult(solutionCount=n, solutions=sols) # pyright: ignore[reportCallIssue] + + +# ───────────── --pick must not name a row the user never saw ───────────── + + +def test_pick_beyond_the_rendered_table_falls_back() -> None: + """The table hardcoded 10 rows while `--pick` validated against the full + solution list, so `-n 15 --pick 15` printed 10 rows and then emitted a + booking link labelled "itinerary #15 pinned" — for a row never displayed, + and with no out-of-range warning because 15 was in range for the + unrendered list.""" + assert _pinned_solution_index(_res(15), 15, 10) == 0 # fell back to cheapest + + +def test_pick_within_the_rendered_table_is_honoured() -> None: + assert _pinned_solution_index(_res(15), 8, 10) == 7 + + +def test_pick_is_honoured_when_the_table_was_widened() -> None: + """`-n 15` renders 15 rows, so `--pick 15` is now legitimate.""" + assert _pinned_solution_index(_res(15), 15, 15) == 14 + + +# ───────────── every seated passenger reaches both backends ───────────── + + +def test_infant_in_seat_counts_toward_the_award_seat_count() -> None: + """An infant IN SEAT buys a seat; only a LAP infant doesn't. Omitting it + made the award query ask for fewer seats than the cash query on the same + run, so an award with too little availability rendered as bookable.""" + p = Pax(adults=2, children=1, infants_in_seat=1, infants_in_lap=1) + assert _seated_pax(p) == 4 + + +def test_lap_infant_does_not_consume_a_seat() -> None: + assert _seated_pax(Pax(adults=1, infants_in_lap=1)) == 1 + + +def _fli_pax(**kw: Any) -> Any: + from flight_cli.fli_bridge import to_fli_filter + + s = SpecificDateSearch( + legs=(Leg(origins=("JFK",), destinations=("LHR",), date=date(2026, 9, 1)),), + options=SearchOptions(pax=Pax(**kw)), + ) + return to_fli_filter(s).passenger_info + + +def test_child_only_search_does_not_synthesize_an_adult() -> None: + """`adults=(...) or 1` invented an adult for a child-only search, pricing a + two-passenger trip nobody asked for. fli permits adults=0.""" + pi = _fli_pax(adults=0, children=1) + assert (pi.adults, pi.children) == (0, 1) + + +def test_infants_reach_the_google_flights_bridge() -> None: + """Infants were dropped entirely on this path, so an infant-in-seat search + priced one fewer seat than the Matrix side of the same run.""" + pi = _fli_pax(adults=2, children=1, infants_in_seat=1, infants_in_lap=1) + assert (pi.adults, pi.children, pi.infants_in_seat, pi.infants_on_lap) == (2, 1, 1, 1) + + +# ───────── the Google link states how it differs from the search ───────── + + +def test_multi_airport_and_routing_search_gets_caveats() -> None: + """`fast_flights`' tfs= format takes one airport pair and has no routing + field, so a multi-airport routed search silently degrades: rows flying + EWR->LGW under `--routing AA+` sat beside a link searching JFK->LHR + unconstrained, while the Matrix link on the same output was faithful.""" + from flight_cli.cli import _gflight_url_caveats + + s = SpecificDateSearch( + legs=( + Leg( + origins=("JFK", "EWR", "LGA"), + destinations=("LHR", "LGW"), + date=date(2026, 9, 1), + route_language="AA+", + extension="f bc=J", + ), + ), + ) + notes = _gflight_url_caveats(s) + assert any("multi-airport" in n and "JFK→LHR" in n for n in notes) + assert any("routing/extension" in n for n in notes) + + +def test_plain_search_gets_no_caveats() -> None: + """A search the link CAN express must not be annotated.""" + from flight_cli.cli import _gflight_url_caveats + + s = SpecificDateSearch( + legs=(Leg(origins=("JFK",), destinations=("LHR",), date=date(2026, 9, 1)),), + ) + assert _gflight_url_caveats(s) == [] + + +# ───────── an award without enough seats is flagged, not hidden ───────── + + +def _award(miles: int, seats: int | None) -> Any: + from flight_cli.providers.base import AwardFlight, CabinAward + + return AwardFlight( + origin="JFK", + destination="LHR", + departure="d", + arrival="a", + flight_number="AA100", + num_connections=0, + provider="Seats.aero", + program="American Airlines", + miles_to_cash_ratio=0.0125, + funding_banks=["Chase"], + cabins=[ + CabinAward( + cabin="Business", + miles=miles, + tax_usd=5.6, + tax_currency="USD", + remaining_seats=seats, + ), + ], + ) + + +def test_award_with_fewer_seats_than_the_party_is_flagged() -> None: + """seats.aero reports RemainingSeats and the provider discarded it, so a + 57,500-mile business award with ONE seat rendered as available to a party + of four.""" + from flight_cli.pp.cli import _fmt_award_cell + + cell = _fmt_award_cell([_award(57_500, 1)], "Business", None, 4) + assert "1 seat" in cell + + +def test_sufficient_seats_are_not_flagged() -> None: + from flight_cli.pp.cli import _fmt_award_cell + + assert "seat" not in _fmt_award_cell([_award(57_500, 4)], "Business", None, 4) + + +def test_unreported_seat_count_is_not_treated_as_zero() -> None: + """None means "not reported" — PointsPath never reports it, and + seats.aero's 0 is usually staleness. Neither is a claim of no seats.""" + from flight_cli.pp.cli import _fmt_award_cell + + assert "seat" not in _fmt_award_cell([_award(57_500, None)], "Business", None, 4) diff --git a/tests/test_gflight_cookie_jar.py b/tests/test_gflight_cookie_jar.py index d7c1193..a0f5c91 100644 --- a/tests/test_gflight_cookie_jar.py +++ b/tests/test_gflight_cookie_jar.py @@ -160,3 +160,52 @@ def test_persist_skips_when_no_allowlisted_cookie( warm = _FakeClient([_JarCookie("AEC", "x", ".google.com")]) # Google but not NID gfid._persist_cookies(warm) assert not (tmp_path / "gflight-cookies.json").exists() + + +# ───────── the jar must resolve against the REAL fli client ───────── + + +def test_cookie_jar_resolves_against_the_installed_fli_client() -> None: + """Regression: this module reached for `Client._client`, which fli 0.9 + replaced with a per-thread `Client._session()`. The AttributeError landed + in a best-effort `except` and was logged at debug, so NID seeding AND + persistence became silent no-ops — every process started cold, against a + documented ~40% cold-start empty rate versus ~0% warm. + + Deliberately exercised against the real installed client rather than a + stub: a stub would have kept passing through the upstream rename, which is + exactly how this went unnoticed. + """ + from fli.search.client import get_client # pyright: ignore[reportMissingTypeStubs] + + from flight_cli._gflight_ids import _cookie_jar + + jar = _cookie_jar(get_client()) # pyright: ignore[reportUnknownArgumentType] + assert hasattr(jar, "set") + assert hasattr(jar, "jar") + + +def test_cookie_jar_seeding_is_observable() -> None: + """A seeded cookie must actually be readable back — the property the + silent no-op destroyed.""" + from fli.search.client import get_client # pyright: ignore[reportMissingTypeStubs] + + from flight_cli._gflight_ids import _cookie_jar + + jar = _cookie_jar(get_client()) # pyright: ignore[reportUnknownArgumentType] + jar.set("NID", "sentinel-value", domain=".google.com", path="/") + names = [str(c.name) for c in jar.jar] + assert "NID" in names + + +def test_cookie_jar_raises_when_no_known_shape() -> None: + """Fails loudly on a future rename instead of degrading to a no-op again.""" + import pytest + + from flight_cli._gflight_ids import _cookie_jar + + class _Alien: + pass + + with pytest.raises(AttributeError): + _ = _cookie_jar(_Alien()) diff --git a/tests/test_http_cache.py b/tests/test_http_cache.py new file mode 100644 index 0000000..cc45495 --- /dev/null +++ b/tests/test_http_cache.py @@ -0,0 +1,159 @@ +# pyright: reportPrivateUsage=false +# DIVERGE: swapping in a MockTransport requires reaching for `_client`, the +# same pattern tests/pp/test_client_request_retry.py established. The public +# constructor has no transport injection point. +"""Tests for HttpTransport's response cache. + +`_http.py` had no dedicated test file despite sitting under every network call +the tool makes. These pin the two properties that actually protect a user: an +application-level error is never memoized, and a cached fare eventually +expires. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING, Any + +import anyio +import httpx + +from flight_cli._http import HttpTransport, is_cacheable_body + +if TYPE_CHECKING: + import pathlib + + +def _transport(tmp: pathlib.Path, handler: Any, **kw: Any) -> HttpTransport: + t = HttpTransport(cache_dir=tmp, rps=1000.0, **kw) + # Swap in a mock transport; the constructor's client has done no I/O yet. + t._client = httpx.AsyncClient(transport=httpx.MockTransport(handler)) + return t + + +def _counting_handler(body: dict[str, Any]) -> tuple[Any, list[int]]: + calls = [0] + + def handler(_req: httpx.Request) -> httpx.Response: + calls[0] += 1 + return httpx.Response(200, json=body) + + return handler, calls + + +# ───────────────────────── error bodies are never stored ───────────────────── + + +def test_matrix_error_envelope_is_not_cacheable() -> None: + """Matrix signals failure with HTTP 200 + `{"error": ...}`, so no + status-code or HTTP-aware rule can catch it — the check needs Matrix's own + shape.""" + assert is_cacheable_body({"solutionList": {"solutions": []}}) is True + assert is_cacheable_body({"error": {"message": "Internal server error."}}) is False + assert is_cacheable_body({"error": {"message": "QPX Warning. Bad route"}}) is False + + +def test_error_response_is_refetched_not_memoized(tmp_path: pathlib.Path) -> None: + """The bug this prevents: a transient brownout became permanent. A live + cache inspected during review held 19 such entries, each replayed forever + because nothing expired.""" + handler, calls = _counting_handler({"error": {"message": "Internal server error."}}) + + async def go() -> None: + t = _transport(tmp_path, handler) + _ = await t.post_json("https://x.test/v1/search", {"q": 1}) + _ = await t.post_json("https://x.test/v1/search", {"q": 1}) + await t.aclose() + + anyio.run(go) + assert calls[0] == 2 # both requests hit the network + + +def test_successful_response_is_served_from_cache(tmp_path: pathlib.Path) -> None: + handler, calls = _counting_handler({"solutionList": {"solutions": [{"id": "a"}]}}) + + async def go() -> None: + t = _transport(tmp_path, handler) + first = await t.post_json("https://x.test/v1/search", {"q": 1}) + second = await t.post_json("https://x.test/v1/search", {"q": 1}) + assert first == second + await t.aclose() + + anyio.run(go) + assert calls[0] == 1 # second request served from cache + + +# ────────────────────────────── entries expire ─────────────────────────────── + + +def test_cached_entry_expires(tmp_path: pathlib.Path) -> None: + """A fare must not be served indefinitely. The previous hand-rolled cache + had no expiry at all: its newest entry was 16 hours old and its oldest 77 + days, all still live.""" + handler, calls = _counting_handler({"solutionList": {"solutions": []}}) + + async def go() -> None: + t = _transport(tmp_path, handler, cache_ttl=0.5) + _ = await t.post_json("https://x.test/v1/search", {"q": 1}) + await anyio.sleep(0.7) + _ = await t.post_json("https://x.test/v1/search", {"q": 1}) + await t.aclose() + + anyio.run(go) + assert calls[0] == 2 + + +def test_distinct_bodies_do_not_share_an_entry(tmp_path: pathlib.Path) -> None: + """Two different searches must never resolve to one cached response.""" + handler, calls = _counting_handler({"solutionList": {"solutions": []}}) + + async def go() -> None: + t = _transport(tmp_path, handler) + _ = await t.post_json("https://x.test/v1/search", {"origin": "MSY"}) + _ = await t.post_json("https://x.test/v1/search", {"origin": "JFK"}) + await t.aclose() + + anyio.run(go) + assert calls[0] == 2 + + +def test_cache_read_disabled_always_refetches(tmp_path: pathlib.Path) -> None: + handler, calls = _counting_handler({"solutionList": {"solutions": []}}) + + async def go() -> None: + t = _transport(tmp_path, handler, cache_read=False) + _ = await t.post_json("https://x.test/v1/search", {"q": 1}) + _ = await t.post_json("https://x.test/v1/search", {"q": 1}) + await t.aclose() + + anyio.run(go) + assert calls[0] == 2 + + +# ──────────────────────── cache key is unambiguous ──────────────────────── + + +def test_param_value_containing_a_separator_cannot_forge_another_key( + tmp_path: pathlib.Path, +) -> None: + """The key flattened params with an unescaped `"&".join(f"{k}={v}")`, so a + VALUE containing a separator produced another request's hash — whichever + ran first served the other its flight data.""" + t = HttpTransport(cache_dir=tmp_path) + injected = t._cache_key("GET", "https://x.test/s", {"a": "1&b=2"}, None) + genuine = t._cache_key("GET", "https://x.test/s", {"a": "1", "b": "2"}, None) + assert injected != genuine + + +def test_get_and_post_never_share_an_entry(tmp_path: pathlib.Path) -> None: + t = HttpTransport(cache_dir=tmp_path) + assert t._cache_key("GET", "https://x.test/s", None, None) != t._cache_key( + "POST", "https://x.test/s", None, None + ) + + +def test_param_order_does_not_change_the_key(tmp_path: pathlib.Path) -> None: + """Same request, different dict ordering, must hit the same entry.""" + t = HttpTransport(cache_dir=tmp_path) + assert t._cache_key("GET", "https://x.test/s", {"b": "2", "a": "1"}, None) == t._cache_key( + "GET", "https://x.test/s", {"a": "1", "b": "2"}, None + ) diff --git a/tests/test_links_gflight_pin.py b/tests/test_links_gflight_pin.py index 1c8c987..2934909 100644 --- a/tests/test_links_gflight_pin.py +++ b/tests/test_links_gflight_pin.py @@ -1,3 +1,7 @@ +# pyright: reportPrivateUsage=false +# DIVERGE: these pin wire-format contracts of the pinned-tfs encoder, which is +# module-internal by design. Exporting it to satisfy the rule would widen the +# API for a test. """Byte-exact regression test for the Google Flights pinned-itinerary `tfs=` protobuf encoder. @@ -15,6 +19,7 @@ from __future__ import annotations import pathlib +from typing import Any from flight_cli.links import ( _encode_gflight_pinned_tfs, # pyright: ignore[reportPrivateUsage] # test-only: lock byte-exact regression @@ -219,3 +224,179 @@ def test_extract_pin_segments_bails_on_missing_data() -> None: stops=[], # 2 flights but 0 stops — invalid topology ) assert extract_pin_segments_from_slice(s) is None + + +# ───────── --pick must index the rows the user actually saw ───────── + + +def test_pin_source_follows_the_rendered_merged_order() -> None: + """`--pick N` names a row from the merged GF+Matrix table. Pinning from + `matrix_res` indexed a DIFFERENT sequence: a Google-only row is cheaper, so + it sorts first and shifts every Matrix row down by one — `--pick 2` then + linked the itinerary shown at row 1. That link is the handoff to a real + booking. + """ + from types import SimpleNamespace + + # DIVERGE: reportPrivateUsage — pinning the row-ordering contract is + # exactly the case for reaching into a module-internal helper; exporting + # it publicly to satisfy the rule would widen the API for a test. + from flight_cli.cli import _pin_source_from_merged # pyright: ignore[reportPrivateUsage] + from flight_cli.models import ( + Itinerary, + ItineraryDetails, + SearchResult, + Slice, + SliceEndpoint, + ) + + def _itin(fn: str) -> Itinerary: + return Itinerary( + displayTotal="USD500.00", + itinerary=ItineraryDetails( + slices=[ + Slice( + flights=[fn], + departure="2026-09-09T06:00:00", + origin=SliceEndpoint(code="MSY"), + destination=SliceEndpoint(code="MIA"), + ), + ], + carriers=[], + ), + ) + + gf_only = _itin("DL300") # cheaper: sorts to merged row 1 + matrix_a = _itin("AA500") + merged = [SimpleNamespace(itinerary=gf_only), SimpleNamespace(itinerary=matrix_a)] + + # Matrix's own list has AA500 first — the ordering that used to be pinned. + src = SearchResult( # pyright: ignore[reportCallIssue] + solutionCount=1, solutions=[matrix_a], session="S", solutionSet="SS" + ) + + pin = _pin_source_from_merged(merged, 10, src) + + flights = [s.itinerary.slices[0].flights[0] for s in pin.solutions if s.itinerary is not None] + assert flights == ["DL300", "AA500"] + # Server-generated identifiers must survive, or the Matrix pinned URL + # silently degrades to a plain deep link. + assert pin.session == "S" + assert pin.solution_set == "SS" + + +# ───────── multi-city and passenger types survive into the pinned link ───────── + + +def _pin_slice(origin: str, dest: str, date: str) -> dict[str, Any]: + return { + "date": date, + "origin": origin, + "destination": dest, + "segments": [ + { + "origin": origin, + "date": date, + "destination": dest, + "carrier": "AA", + "flight": "100", + }, + ], + } + + +def _field8_values(buf: bytes) -> list[int]: + """Every top-level field-8 varint (tag byte 0x40) — the per-occupant types.""" + out: list[int] = [] + i = 0 + while i < len(buf): + if buf[i] == 0x40: + out.append(buf[i + 1]) + i += 2 + else: + i += 1 + return out + + +def test_passenger_types_are_not_all_encoded_as_adults() -> None: + """Field 8 carries each occupant's TYPE (Google's Passenger enum, read from + fast_flights' generated protobuf: ADULT=1, CHILD=2, INFANT_IN_SEAT=3, + INFANT_ON_LAP=4). Emitting a bare 1 for everyone made a pinned link for + 1 adult + 1 child search and price as 2 adults — a different, costlier + itinerary than the row the user picked.""" + from flight_cli.links import _encode_gflight_pinned_tfs # pyright: ignore[reportPrivateUsage] + + buf = _encode_gflight_pinned_tfs( + slices=[_pin_slice("SFO", "JFK", "2026-09-01")], + cabin=1, + adults=1, + children=1, + infants_in_seat=1, + infants_on_lap=1, + ) + assert _field8_values(buf) == [1, 2, 3, 4] + + +def test_two_adults_still_encode_as_two_adults() -> None: + from flight_cli.links import _encode_gflight_pinned_tfs # pyright: ignore[reportPrivateUsage] + + buf = _encode_gflight_pinned_tfs( + slices=[_pin_slice("SFO", "JFK", "2026-09-01")], + cabin=1, + adults=2, + children=0, + infants_in_seat=0, + infants_on_lap=0, + ) + assert _field8_values(buf) == [1, 1] + + +def test_three_leg_itinerary_is_multi_city_not_round_trip() -> None: + """`>= 2 slices` meant round-trip, so Google read only the first two and + leg 3 vanished from a link still described as "pinned".""" + from flight_cli.links import ( # pyright: ignore[reportPrivateUsage] + _GF_TRIP_MULTI_CITY, + _GF_TRIP_ONE_WAY, + _GF_TRIP_ROUND_TRIP, + _encode_gflight_pinned_tfs, + ) + + def trip_type(n_slices: int) -> int: + route = [("SFO", "JFK"), ("JFK", "LHR"), ("LHR", "SFO")][:n_slices] + buf = _encode_gflight_pinned_tfs( + slices=[_pin_slice(o, d, "2026-09-01") for o, d in route], + cabin=1, + adults=1, + children=0, + infants_in_seat=0, + infants_on_lap=0, + ) + return buf[-1] # field 19 is the last varint written + + assert trip_type(1) == _GF_TRIP_ONE_WAY + assert trip_type(2) == _GF_TRIP_ROUND_TRIP + assert trip_type(3) == _GF_TRIP_MULTI_CITY + + +def test_stop_limit_reaches_the_google_search_url() -> None: + """`max_stops` is a TFSData-level field, not per-FlightData. Omitting it + made a `--stops 0` link byte-identical to an unconstrained one, so a + nonstop-only result table handed the user a page that also offered + connections.""" + from datetime import date + + from flight_cli.domain import Leg, SearchOptions, SpecificDateSearch + from flight_cli.links import google_flights_url + + def url(max_extra_stops: int | None) -> str: + return google_flights_url( + SpecificDateSearch( + legs=(Leg(origins=("JFK",), destinations=("LHR",), date=date(2026, 9, 1)),), + options=SearchOptions(max_extra_stops=max_extra_stops), + ), + ) + + nonstop, one_stop, unconstrained = url(0), url(1), url(None) + assert nonstop != unconstrained + assert nonstop != one_stop + assert one_stop != unconstrained diff --git a/tests/test_links_matrix_url.py b/tests/test_links_matrix_url.py index c613ba8..d73a3ee 100644 --- a/tests/test_links_matrix_url.py +++ b/tests/test_links_matrix_url.py @@ -1,3 +1,7 @@ +# pyright: reportPrivateUsage=false +# DIVERGE: these pin wire-format contracts of the SPA slice builder, which is +# module-internal by design. Exporting it to satisfy the rule would widen the +# API for a test. """Regression tests for `matrix_deep_link()` — the SPA URL-state encoder. Round-trip ground truth was captured 2026-05-19 by driving Matrix's SPA via @@ -28,7 +32,8 @@ SearchOptions, SpecificDateSearch, ) -from flight_cli.links import matrix_deep_link, matrix_itinerary_url +from flight_cli.links import extract_pin_segments_from_slice, matrix_deep_link, matrix_itinerary_url +from flight_cli.models import Slice, SliceEndpoint FIXTURE_DIR = pathlib.Path(__file__).parent / "fixtures" / "matrix_url" @@ -174,3 +179,193 @@ def test_multi_city_keeps_one_slice_per_leg() -> None: assert dates["departureDate"] == iso # Multi-city slices keep returnDate empty — our benign superset. assert dates["returnDate"] == "" + + +# ───────── pinned segment dates: a segment is dated by when it DEPARTS ───────── + + +def _slice( + flights: list[str], dep: str, arr: str, o: str, d: str, stops: list[str] | None = None +) -> Slice: + return Slice( + flights=flights, + departure=dep, + arrival=arr, + origin=SliceEndpoint(code=o), + destination=SliceEndpoint(code=d), + stops=[SliceEndpoint(code=c) for c in (stops or [])], + ) + + +def test_overnight_nonstop_is_dated_by_departure() -> None: + """A nonstop satisfies `i == n - 1`, so the last-segment rule dated it by + ARRIVAL — pinning BA178 JFK->LHR (dep 2026-12-31, arr 2027-01-01) to + 2027-01-01 and sending the user to a search for the wrong day, across a + year boundary.""" + segs = extract_pin_segments_from_slice( + _slice(["BA178"], "2026-12-31T22:00-05:00", "2027-01-01T10:00+00:00", "JFK", "LHR"), + ) + assert segs is not None + assert [x["date"] for x in segs] == ["2026-12-31"] + + +def test_overnight_connection_still_dates_its_last_leg_by_arrival() -> None: + """The rule the nonstop case was over-applying is real for a genuine + connection: the second leg does depart on the following day.""" + segs = extract_pin_segments_from_slice( + _slice( + ["AA100", "BA200"], + "2026-12-31T18:00-05:00", + "2027-01-01T14:00+00:00", + "JFK", + "LHR", + ["BOS"], + ), + ) + assert segs is not None + assert [x["date"] for x in segs] == ["2026-12-31", "2027-01-01"] + + +def test_same_day_nonstop_unchanged() -> None: + segs = extract_pin_segments_from_slice( + _slice(["AA867"], "2026-09-09T06:00-05:00", "2026-09-09T09:04-04:00", "MSY", "MIA"), + ) + assert segs is not None + assert [x["date"] for x in segs] == ["2026-09-09"] + + +# ───────── a 2-leg search is only a round trip if it actually returns ───────── + + +def test_open_jaw_is_multi_city_not_a_collapsed_round_trip() -> None: + """Round-trip's SPA encoding folds both legs into ONE slice with two dates, + which cannot express a second route. Treating any 2-leg search as a round + trip therefore DELETED the second leg: SFO->JFK plus LAX->HNL encoded as + SFO->JFK with a return date, and LAX/HNL vanished entirely.""" + from flight_cli.domain import Leg + from flight_cli.links import _spa_specific_slices # pyright: ignore[reportPrivateUsage] + + legs = ( + Leg(origins=("SFO",), destinations=("JFK",), date=date(2026, 9, 1)), + Leg(origins=("LAX",), destinations=("HNL",), date=date(2026, 9, 5)), + ) + trip, slices = _spa_specific_slices(legs) + assert trip == "multi-city" + assert [(s["origin"][0], s["dest"][0]) for s in slices] == [("SFO", "JFK"), ("LAX", "HNL")] + + +def test_true_round_trip_still_folds_into_one_slice() -> None: + from flight_cli.domain import Leg + from flight_cli.links import _spa_specific_slices # pyright: ignore[reportPrivateUsage] + + legs = ( + Leg(origins=("SFO",), destinations=("JFK",), date=date(2026, 9, 1)), + Leg(origins=("JFK",), destinations=("SFO",), date=date(2026, 9, 5)), + ) + trip, slices = _spa_specific_slices(legs) + assert trip == "round-trip" + assert len(slices) == 1 + assert slices[0]["dates"]["returnDate"] == "2026-09-05" + + +def test_half_open_jaw_is_multi_city() -> None: + """Returns from where it landed but not to where it started.""" + from flight_cli.domain import Leg + from flight_cli.links import _spa_specific_slices # pyright: ignore[reportPrivateUsage] + + legs = ( + Leg(origins=("SFO",), destinations=("JFK",), date=date(2026, 9, 1)), + Leg(origins=("JFK",), destinations=("LAX",), date=date(2026, 9, 5)), + ) + trip, _ = _spa_specific_slices(legs) + assert trip == "multi-city" + + +# ───────── routing / extension / arrival-date reach the deep link ───────── + + +def _decoded(s: object) -> dict[str, Any]: + import base64 + import json + import urllib.parse + + from flight_cli.links import matrix_deep_link + + q = urllib.parse.unquote(matrix_deep_link(s).split("search=", 1)[1]) # pyright: ignore[reportArgumentType] + return json.loads(base64.b64decode(q + "==")) + + +def test_routing_and_extension_reach_the_matrix_link() -> None: + """Field names captured from the real SPA (2026-08): the URL state calls + these `routing` / `ext`, NOT the `routeLanguage` / `commandLine` the /batch + API uses for the same values — see docs/memories/matrix_spa_url_state.md. + + Dropping them meant a link built from `--routing BA+ --ext "MAXSTOPS 0"` + opened an UNCONSTRAINED search, showing itineraries the CLI had excluded. + """ + from flight_cli.domain import Leg, SpecificDateSearch + + s = SpecificDateSearch( + legs=( + Leg( + origins=("JFK",), + destinations=("LHR",), + date=date(2026, 9, 1), + route_language="BA+", + extension="MAXSTOPS 0", + ), + ), + ) + sl = _decoded(s)["slices"][0] + assert sl["routing"] == "BA+" + assert sl["ext"] == "MAXSTOPS 0" + + +def test_return_leg_carries_its_own_routing_codes() -> None: + """The SPA folds a round trip into one slice, so the inbound leg's codes + live in the separate `routingRet` / `extRet` fields.""" + from flight_cli.domain import Leg, SpecificDateSearch + + s = SpecificDateSearch( + legs=( + Leg( + origins=("JFK",), + destinations=("LHR",), + date=date(2026, 9, 1), + route_language="BA+", + extension="MAXSTOPS 0", + ), + Leg( + origins=("LHR",), + destinations=("JFK",), + date=date(2026, 9, 8), + route_language="AA+", + extension="MAXCONNECT 2:00", + ), + ), + ) + sl = _decoded(s)["slices"][0] + assert (sl["routing"], sl["ext"]) == ("BA+", "MAXSTOPS 0") + assert (sl["routingRet"], sl["extRet"]) == ("AA+", "MAXCONNECT 2:00") + + +def test_arrival_date_intent_survives_into_the_link() -> None: + """The URL-state counterpart of the API's `isArrivalDate` bool is the + string `departureDateType: "arrive"`.""" + from flight_cli.domain import Leg, SpecificDateSearch + + def date_type(is_arrival: bool) -> str: + s = SpecificDateSearch( + legs=( + Leg( + origins=("JFK",), + destinations=("LHR",), + date=date(2026, 9, 1), + is_arrival_date=is_arrival, + ), + ), + ) + return _decoded(s)["slices"][0]["dates"]["departureDateType"] + + assert date_type(True) == "arrive" + assert date_type(False) == "depart" diff --git a/uv.lock b/uv.lock index 47e77e1..eb42d51 100644 --- a/uv.lock +++ b/uv.lock @@ -267,6 +267,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/59/8c/36bbe06d66fa2b765e4a07199f643a59a9cd1a754207a96335402a9520f4/curl_cffi-0.15.0-cp314-cp314t-win_arm64.whl", hash = "sha256:0b6c0543b993996670e9e4b78e305a2d60809d5681903ffb5568e21a387434d3", size = 1466312, upload-time = "2026-04-03T11:12:30.054Z" }, ] +[[package]] +name = "diskcache" +version = "5.6.3" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/3f/21/1c1ffc1a039ddcc459db43cc108658f32c57d271d7289a2794e401d0fdb6/diskcache-5.6.3.tar.gz", hash = "sha256:2c3a3fa2743d8535d832ec61c2054a1641f41775aa7c556758a109941e33e4fc", size = 67916, upload-time = "2023-08-31T06:12:00.316Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/3f/27/4570e78fc0bf5ea0ca45eb1de3818a23787af9b390c0b0a0033a1b8236f9/diskcache-5.6.3-py3-none-any.whl", hash = "sha256:5e31b2d5fbad117cc363ebaf6b689474db18a1f6438bc82358b024abd4c2ca19", size = 45550, upload-time = "2023-08-31T06:11:58.822Z" }, +] + [[package]] name = "fast-flights" version = "2.2" @@ -288,6 +297,7 @@ source = { editable = "." } dependencies = [ { name = "aiolimiter" }, { name = "anyio" }, + { name = "diskcache" }, { name = "fast-flights" }, { name = "flights" }, { name = "httpx" }, @@ -319,6 +329,7 @@ dev = [ requires-dist = [ { name = "aiolimiter", specifier = ">=1.1" }, { name = "anyio", specifier = ">=4" }, + { name = "diskcache", specifier = ">=5.6" }, { name = "fast-flights", specifier = ">=2.2" }, { name = "flights", specifier = ">=0.9" }, { name = "httpx", specifier = ">=0.27" },