Skip to content

Adopt python-starter conventions#2

Merged
ak2k merged 9 commits into
mainfrom
migrate-to-python-starter
May 15, 2026
Merged

Adopt python-starter conventions#2
ak2k merged 9 commits into
mainfrom
migrate-to-python-starter

Conversation

@ak2k

@ak2k ak2k commented May 15, 2026

Copy link
Copy Markdown
Owner

Summary

Migrates flight-cli to the ak2k/python-starter template (Profile A — distributable CLI, with a Profile-B edge for the reverse-engineered boundaries). flight-cli is the first real-world adopter of the template; this PR validates the template against an existing 2K-LOC repo and surfaces the divergences that are genuinely justified vs. those that were laziness.

Stack now matches the template

Every row in AGENTS.md's "Stack" table is now ✓:

Concern Use
Package manager uv (uv.lock now tracked)
Lint / format ruff strict ruleset
Type checker basedpyright strict (was pyright; pyrightconfig.json folded into pyproject)
Boundary validation Pydantic v2
HTTP httpx (+ httpx-curl-cffi transport for fingerprinting)
Async runtime anyio (was raw asyncio)
Logging structlog (was stdlib logging + RichHandler)
Tests pytest + hypothesis (property tests added for domain validators)

Inner-loop scaffolding

  • Makefile (make check / make fix), AGENTS.md (Profile A flavored with canonical-example pointers into the existing source), .claude/settings.json allowlist, py.typed, .editorconfig, .python-version.
  • flake.nix derives the Python interpreter from requires-python via pyproject-nix's lib.project.loadPyproject — bumping the floor auto-swaps the dev shell.
  • CI: ci.yml + gitleaks.yml. uv path runs unconditionally; nix path activates because flake.nix is present.

Justified divergences (Profile-A→A/B edge)

flight-cli sits between Profile A (CLI/lib) and Profile B (RE/scraping). Documented in AGENTS.md's "Appropriate divergence" table with rationale:

  • requires-python >= 3.12 (vs template >= 3.13)
  • Ruff D* (docstrings) dropped; PLR0913 ignored (CLI verbs are wide)
  • N815 per-file-ignored on wire.py only (camelCase mirrors Matrix's JSON shape directly without per-field alias=)
  • reportAny = "none" — fli/fast_flights ship no stubs; Matrix response is dict[str, Any] by design; reportUnknown* rules stay on (the higher-signal "can't type this" warnings)
  • Pydantic extra="ignore" on wire + response models (Matrix is reverse-engineered and adds fields unannounced in directions that aren't load-bearing). Domain models stay extra="forbid" — that's where we control the contract.
  • Coverage gate = 0 (golden-file regression tests, not coverage-driven)

Code adjustments to satisfy the strict ruleset

All behavior-preserving: http.HTTPStatus.* enum constants in place of bare HTTP status codes; StrEnum in place of (str, Enum); narrowed bare excepts with from e re-raises; contextlib.suppress for ignored OSError; named-constant sentinels (_ROUND_TRIP_LEGS, _IATA_CODE_LEN, _SLICE_*_PARTS); cast() in place of three isinstance(...) asserts; lazy fli imports kept with # noqa: PLC0415 + reason.

UX polish

  • Currency renders once in table titles (Itineraries (USD), Carrier x stops grid (USD)) instead of USD repeated on every cell.

Property tests

tests/test_domain_properties.py adds 9 hypothesis tests for domain.py validators (_iata, Leg.of, CalendarWindow, Pax.total, time_range_for, SpecificDateSearch.legs). Caught a real mental-model bug during authorship: my filter for "non-IATA garbage" assumed strict casing, but _iata() .upper().strip()s before validating, so "AAa""AAA" is accepted. Fixed the test (not the validator) to match actual behavior.

Test plan

  • make check is 0 errors / 0 warnings / 12 tests passing
  • Live flight fare JFK LHR --dep 2026-08-15 --return 2026-08-22 -n 2 returns 64 solutions, cheapest USD877 VS154 (output identical before/after migration)
  • flight -vv fare … emits structlog ConsoleRenderer events on stderr (cache_hit method=POST url=…)
  • Currency surfaces once in table titles; cells show just the amount
  • CI green on this PR

Commits (8)

002b5f5 Adopt python-starter conventions (basedpyright, ruff strict, AGENTS.md, CI)
3688b14 Silence reportAny: declare Profile-B edge for untyped boundaries
d6d750a Adopt anyio + add flake.nix
382e41e AGENTS.md: response models are extra="ignore", not strict
4c032a5 Render currency once in table titles, not on every cell
010ddb1 Adopt structlog: drop stdlib logging + RichHandler
c56ef2e Add hypothesis property tests for domain validators
38d0e35 Drop dead N815 per-file-ignores from domain.py and models.py

Reviewable commit-by-commit; each is independently scoped.

ak2k added 9 commits May 15, 2026 00:10
…d, CI)

Migrates flight-cli to the ak2k/python-starter template (Profile A —
distributable CLI). Validates the template against a real 2K-LOC repo;
the gaps to close were narrow because the runtime stack already matched
(src/ + httpx + Pydantic + typer + stamina + aiolimiter + httpx-curl-cffi).

Changes:

* Type checker: pyright --strict → basedpyright strict; pyrightconfig.json
  folded into [tool.basedpyright]. 0 errors on src/ and tests/; the
  remaining reportAny warnings sit at typed-as-Any third-party boundaries
  (fli, fast_flights, raw JSON) and are expected per template.

* Lint: full python-starter ruff ruleset (E/W/F/I/B/UP/SIM/RUF/ASYNC/PTH/
  TC/RET/PIE/PL/ANN/S/TRY/BLE/N). N815 (camelCase) per-file-ignored for
  wire.py / domain.py / models.py — Matrix's JSON dictates field names.
  Profile A divergences (D*, PLR0913, fail_under=0) commented at site.

* Inner-loop scaffolding: Makefile, AGENTS.md (Profile A flavored, with
  canonical-example pointers into the existing source instead of new
  example files), .claude/settings.json allowlist, py.typed, .editorconfig,
  .python-version. .gitignore tracks .claude/settings.json now (template
  convention — discoverable allowlist for new collaborators).

* CI: ci.yml + gitleaks.yml ported from template. uv path runs for
  everyone; conditional nix path is a no-op without flake.nix.

* Code adjustments to satisfy strict ruleset (all behavior-preserving):
  HTTPStatus enum in place of bare 200/403/404/408/500 magic numbers,
  StrEnum in place of (str, Enum), narrowed bare excepts, `from e` on
  re-raises, contextlib.suppress for ignored OSError, _ROUND_TRIP_LEGS /
  _IATA_CODE_LEN / _SLICE_*_PARTS sentinels for clarity, lazy fli
  imports kept with explicit noqa + reason. cli.py loses three
  isinstance asserts in favor of cast (still type-safe via discriminated
  dispatch in client._parse_response).

* uv.lock now tracked (template step 1: commit uv.lock).

`make check` is fully green; `flight fare JFK LHR --dep ... -n 2` smoke
test against the live Alkali backend confirms no runtime regression.
flight-cli is a CLI veneer over RE/scraping work — fli/fast_flights ship
no type stubs, Matrix's JSON response is dict[str, Any] by design, and
pydantic's ValidationInfo.data + Field overloads are Any internally.
All 62 reportAny warnings sat at these unfixable boundaries.

Keep all reportUnknown* rules at their strict defaults (those are the
higher-signal "can't type this" warnings, and they already caught real
gaps during the strict-mode migration). Silence reportAny only.

`make check`: 0 errors, 0 warnings.

Also: hoist the reportMissingTypeStubs ignore to the from-import line
in fli_bridge.run_gflight_search (the per-symbol placement only
suppressed the symbol-level miss, not the module-level one).
Drops two divergences that weren't actually justified:

* asyncio → anyio. Surface was tiny (asyncio.Semaphore at _http.py:90,
  asyncio.run at cli.py:195,722). httpx is anyio-internal under the
  hood, so we were bypassing the runtime our HTTP client already speaks.
  anyio's CancelScope semantics also clean up Ctrl-C behavior during
  long Matrix searches. ~5 lines of code change.

* Add flake.nix. Template-provided, derives Python interpreter from
  pyproject.toml's `requires-python` via pyproject-nix.lib.project.
  loadPyproject — bumping the floor auto-swaps the dev shell. Also
  activates the conditional nix path in CI (.github/workflows/ci.yml's
  `hashFiles('flake.nix') != ''` block), restoring reproducibility
  verification on every PR.

AGENTS.md updated: stack table flips async row to ✓, principle 8 now
documents the anyio idiom, asyncio divergence note removed from
principle 4.

`make check`: 0/0. Live `flight fare JFK LHR --dep ... -n 2` smoke
test under the anyio runtime returns identical output (64 solutions,
cheapest USD877 VS154).
Principle 1 claimed response models in models.py "validate strictly".
They don't — _Loose base uses extra="ignore", same as wire models, for
the same Profile-B reason (Matrix is reverse-engineered and adds
fields unannounced in directions that aren't load-bearing).

Adds a row to the Appropriate-divergence table documenting the
explicit carveout the template anticipates: "Pydantic at every
external edge with extra='forbid' unless an explicit comment
justifies otherwise." This is the textbook justified case.

Domain models in domain.py keep extra="forbid" — that's where we
control the contract.
Matrix returns prices as 'USD877.00' (ISO-4217 prefix + decimal).
Within a single query the currency is uniform, so showing 'USD' on
every grid cell, itinerary row, and calendar day is noise.

Now the currency renders once in the table title ('Itineraries (USD)',
'Carrier x stops grid (USD)', 'lowest fare per departure day (USD)')
and the header line ('cheapest: 877.00 (USD)'). Cells show just the
amount.

Helpers: _split_price(s) -> (currency, amount), _amount(s) -> str.
Both pass through placeholders like '—' unchanged.
Template parity. The four log sites in _http.py (cache_read_failed,
cache_write_failed, cache_hit GET/POST) now emit key-value events
through structlog's ConsoleRenderer; CLI `-v`/`-vv` selects info/debug.

* New src/flight_cli/log.py — idempotent configure(level) with
  TimeStamper + ConsoleRenderer; colors auto-disable on non-TTY.
* _http.py: structlog.get_logger(__name__) typed as BoundLogger via
  if TYPE_CHECKING (per AGENTS.md gotcha #1); printf-style calls
  rewritten as kv events.
* cli.py main(): logging.basicConfig + RichHandler removed; replaced
  with configure_logging("warning"/"info"/"debug").
* structlog added as explicit dep.
* AGENTS.md: Stack table flips Logging to ✓; canonical-examples list
  gains a Logging-config entry; divergence note for stdlib-logging
  removed from principle 4.

`-vv` output: `01:47:31 [debug    ] cache_hit method=POST url=...`
Cleaner than the old `[01:47:31] DEBUG cache hit POST https://...`,
and now bindable with contextvars for future request-correlation.
Complements the wire-fidelity golden-file tests with input-space
exploration of domain.py invariants. Catches typos in alias mappings,
off-by-one errors in length checks, regressions on field_validator
ordering — bugs the regression suite fundamentally can't see because
it tests one captured fixture at a time.

Targets:

* _iata accept/reject — 3-letter alpha after .upper().strip()
* Leg.of — multi-airport lists round-trip
* CalendarWindow — duration_max >= duration_min always enforced
* Pax.total — math holds across all six pax fields
* time_range_for — every TimeOfDay enum value yields valid {min, max}
* SpecificDateSearch.legs — dated/dateless legs validate as expected

Found during authorship: hypothesis surfaced that my test filter for
"non-IATA garbage" was wrong — _iata() .upper().strip()s before
validating, so "AAa" → "AAA" is accepted. Fixed the strategy filter
(not the validator) to match actual behavior. Exactly the class of
mental-model bug property tests catch.

12 tests pass in 0.31s. hypothesis was already in dev-deps.
Audit: only wire.py actually triggers N815 (14 hits) — all on
camelCase pydantic fields that mirror Matrix's JSON shape directly.

domain.py is snake_case throughout (our contract, not Matrix's).
models.py uses snake_case Python attrs + `Field(alias="bookingCode")`
strings — the alias is a string, not an attribute name, so N815
doesn't fire.

Tightened pyproject.toml + AGENTS.md divergence-table row to reflect
the actual single-file scope.
CI's nix-path job failed during pytest collection:

    ImportError: libstdc++.so.6: cannot open shared object file

curl_cffi (via httpx-curl-cffi, used for TLS fingerprinting against
Matrix) ships a compiled C++ extension that dlopens libstdc++.so.6 at
runtime. The Nix devShell doesn't expose the host's library paths,
and python's CFFI binding can't find the symbol.

Fix: add pkgs.stdenv.cc.cc.lib to LD_LIBRARY_PATH inside the
shellHook, scoped to Linux only (Darwin uses libc++ via @rpath and
needs no help). Verified locally — Darwin shell unchanged (empty
LD_LIBRARY_PATH), `nix develop -c make check` runs all 12 tests.
@ak2k ak2k merged commit fb0f5bd into main May 15, 2026
2 checks passed
@ak2k ak2k deleted the migrate-to-python-starter branch May 15, 2026 06:36
ak2k added a commit that referenced this pull request May 15, 2026
After rebasing onto main's adopted conventions, the pp/* and tests/pp/*
code needed a localized cleanup pass to satisfy the strict ruleset.
All behavior-preserving:

* Logging: stdlib `logging.getLogger` → `structlog.get_logger` typed as
  BoundLogger via TYPE_CHECKING. Printf-style log calls rewritten as
  key-value events (e.g. `log.warning("pp_airline_search_failed",
  airline=..., status=..., body=...)`).

* Async: asyncio.Semaphore → anyio.Semaphore; asyncio.gather → anyio
  TaskGroup (cleaner per-task error capture); asyncio.run → anyio.run
  via a closure for kwargs-bearing _gather_pp call.

* HTTP boundaries: bare 200/204/400/401 → http.HTTPStatus.* enum
  constants.

* Pydantic config: pp.models._Loose flipped extra="allow" →
  extra="ignore" to match the main models._Loose Profile-B-edge
  convention (PointsPath is reverse-engineered, same justification as
  Matrix responses).

* pp/models.py added to pyproject.toml's N815 per-file-ignore — mirrors
  PointsPath JSON shape with camelCase attrs (same shape as wire.py).

* Imports: app-local types used only in annotations moved to
  TYPE_CHECKING in match.py / client.py / cli.py. Exception: `Path`
  stays at runtime in pp/cli.py — typer evaluates Annotated[Path|None,
  typer.Option(...)] at runtime, so TYPE_CHECKING breaks the CLI.

* Annotations: bare `dict` → `dict[str, Any]` via _JsonDict aliases;
  added types to test parametrize fixtures; B904 `from e` on re-raises;
  HTTPStatus import.

* Strict shims: `# pyright: reportPrivateUsage=false` on tests/pp/
  test_cli.py (intentional access to _normalize_cabin/_parse_cash/
  _parse_csv); `# pyright: reportCallIssue=false` on tests/pp/
  test_match.py (pydantic Field-with-alias confuses the constructor-
  kwargs check despite populate_by_name=True).

* cli.py: reuses the existing _ROUND_TRIP_LEGS constant; lazy fli/
  fast_flights imports kept.

* Unicode: ambiguous `×` replaced with `x` in docstrings + titles to
  satisfy RUF001/RUF002 (consistent with the Carrier-x-stops convention
  PR #2 established).

`make check`: 0 errors / 0 warnings / 69 tests passing. Live `flight
fare JFK LHR` smoke test green (10 solutions, anyio runtime,
ConsoleRenderer log output on stderr).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant