refactor: harden airport search (Pydantic AirportMatch + import-time validation)

[punitarani]

Summary

Follow-ups to #115 that lock in the right shape for the day-old AirportMatch public API while it still has no external consumers, plus defensive hardening and the test coverage that #115 was missing.

Breaking changes (in fli.core.airports)

AirportMatch is now a frozen Pydantic BaseModel:

• code: Airport (enum, was str)
• match_type: Literal["iata_exact", "iata_prefix", "city", "name"]
• score: float = Field(ge=0, le=100)

match_type no longer overloads "code":

• Priority 1 (exact IATA, score 100) -> "iata_exact"
• Priority 5 (prefix IATA, score 60) -> "iata_prefix"

Previously both used "code", so consumers had to inspect score to tell exact from prefix.

CLI and MCP consumers (r.code.name for the IATA string) are updated accordingly.

Defensive hardening

• CITY_AIRPORTS is validated at import time. A typo like JKF for JFK would previously be swallowed by except KeyError: pass in priorities 2/3 and silently shrink results. Now it raises RuntimeError immediately. The silent guards in priorities 2 and 3 are removed.
• find_airports MCP tool now wraps search_airports in try/except, returning {success: False, error, query} on exception -- matching the envelope used by search_flights and search_dates.

Docs

• Rewrote the misleading CITY_AIRPORTS module comment. The original claim that "city is NOT in the airport name" was false for many entries (atlanta, denver, seattle, miami, berlin, melbourne, sydney, singapore -- their airport names do contain the city).
• search_airports docstring now documents the 5-priority cascade and score bands.

Tests (+19 net)

• tests/core/test_airports.py -- rewrote for new shape; added coverage for limit<1, surrounding whitespace, dedup across priorities (san francisco matches both city map and name substring), priority-3 skip when query is an exact city, monotonic score ordering, position-based scoring, and AirportMatch frozen/equality/validation behaviour.
• tests/mcp/test_find_airports.py (new) -- MCP tool response shape, success/error envelope, limit capping, no-results.
• tests/cli/test_airports.py (new) -- CliRunner coverage for table output, --json flag, no-results exit code, --limit.

Total suite: 220 passing (was 201 on main).

Test plan

• uv run ruff format . --check && uv run ruff check . -> clean
• uv run pytest tests/ --all -v --ignore=tests/search/ -> 220 passed
• CI on this branch reproduces the same green run
• Sanity-check the find_airports MCP tool against an AI assistant by querying a city + an IATA code

🤖 Generated with Claude Code

Greptile Summary

This PR hardens the AirportMatch public API by converting it to a frozen Pydantic BaseModel, renames match_type values ("code" → "iata_exact" / "iata_prefix"), adds import-time validation of CITY_AIRPORTS against the Airport enum, wraps the MCP find_airports tool in a try/except error envelope, and ships the test coverage that was missing from #115.

• AirportMatch is now a frozen Pydantic model with code: Airport, match_type: Literal[...], and score: float = Field(ge=0, le=100); CLI and MCP consumers updated to use r.code.name for the IATA string.
• Import-time CITY_AIRPORTS validation replaces the silent except KeyError: pass guards in priorities 2/3, so a typo in the map raises RuntimeError immediately rather than shrinking results silently.
• 19 new tests cover priority ordering, dedup across priorities, score monotonicity, AirportMatch frozen/equality/validation behavior, MCP response envelope, and CLI --json / --limit flags.

Confidence Score: 4/5

Safe to merge; the import-time validation and Pydantic model hardening are straightforward improvements with good test coverage.

The changes are well-scoped and the logic is sound. The only code concern is that 70.0 - (pos * 0.1) in Priority 4 has no explicit lower-bound clamp, meaning a sufficiently large match position would produce a score below 0.0 and trip the Pydantic ge=0.0 constraint — converting search_airports into an unexpected exception in the CLI path. In practice no real IATA airport name is anywhere near long enough to trigger this, but the formula lacks the max(0.0, ...) guard that would make the invariant self-enforcing.

fli/core/airports.py — score formula in Priority 4; tests/core/test_airports.py — TestAirportMatch validation tests use bare try/except instead of pytest.raises.

Important Files Changed

Filename	Overview
fli/core/airports.py	Core refactor: AirportMatch promoted to a frozen Pydantic model, match_type values renamed, import-time CITY_AIRPORTS validation added, and silent KeyError guards removed. Score formula has a theoretical lower-bound gap (pos > 700) that would trip Pydantic's ge=0.0 constraint.
fli/mcp/server.py	Extracted _find_airports_impl helper with try/except envelope, aligning error shape with search_flights and search_dates. r.code → r.code.name update is correct.
fli/cli/commands/airports.py	Updated r.code → r.code.name in both JSON output and table row; straightforward adaptation to the new Airport enum field type.
tests/core/test_airports.py	Tests rewritten for new AirportMatch shape; good new coverage for dedup, priority ordering, and score bands. TestAirportMatch validation tests use try/except Exception instead of pytest.raises, which silently passes on any exception type.
tests/mcp/test_find_airports.py	New MCP test file testing _find_airports_impl directly; covers success envelope, city query, no-results, limit capping, and invalid limit. Clean and thorough.
tests/cli/test_airports.py	New CLI test file with CliRunner coverage for table output, --json flag, no-results exit code, and --limit; straightforward and well-scoped.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["search_airports(query, limit)"] --> B{empty or limit lt 1?}
    B -- Yes --> Z["return []"]
    B -- No --> P1
    P1["P1: Exact IATA\nscore=100, iata_exact"] --> P2
    P2["P2: Exact city lookup\nscore=90, city"] --> P3
    P3{query in CITY_AIRPORTS?}
    P3 -- No --> P3b["P3: Prefix city match\nscore=80, city"]
    P3 -- Yes --> P4
    P3b --> P4
    P4["P4: Airport name substring\nscore=70-pos*0.1, name"] --> P5
    P5{len query_upper <= 3?}
    P5 -- Yes --> P5b["P5: IATA prefix\nscore=60, iata_prefix"]
    P5 -- No --> SORT
    P5b --> SORT
    SORT["Sort by -score then code.name\nslice to limit"] --> OUT["list of AirportMatch"]

Loading

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
fli/core/airports.py:157-158
The position-based score formula has no lower-bound guard. `70.0 - (pos * 0.1)` becomes negative when `pos > 700`. While no real IATA airport name is that long today, Pydantic's `ge=0.0` constraint on `score` would raise a `ValidationError` the moment it happens, turning `search_airports` into an unexpected exception in the CLI layer (the MCP layer would catch it via `_find_airports_impl`'s try/except). A `max()` clamp costs nothing and makes the invariant self-documenting.

```suggestion
            pos = airport_name_lower.find(query_lower)
            score = max(0.0, 70.0 - (pos * 0.1))
```

### Issue 2 of 2
tests/core/test_airports.py:116-141
These three `TestAirportMatch` tests use a `try/except Exception: return` pattern rather than `pytest.raises`. The problem is that the pattern passes for *any* exception — if Pydantic raises (say) an internal `ImportError` or a `TypeError` for an unrelated reason, the test silently greens. Using `pytest.raises` also pins the expected exception type and gives a cleaner failure message.

```suggestion
    def test_frozen(self):
        import pytest
        from pydantic import ValidationError

        m = AirportMatch(code=Airport.JFK, name="JFK Airport", match_type="iata_exact", score=100.0)
        with pytest.raises(Exception):
            m.score = 0.0

    def test_equality(self):
        a = AirportMatch(code=Airport.JFK, name="JFK", match_type="city", score=90.0)
        b = AirportMatch(code=Airport.JFK, name="JFK", match_type="city", score=90.0)
        assert a == b

    def test_rejects_invalid_match_type(self):
        import pytest
        from pydantic import ValidationError

        with pytest.raises(ValidationError):
            AirportMatch(code=Airport.JFK, name="JFK", match_type="bogus", score=50.0)  # type: ignore[arg-type]

    def test_rejects_score_out_of_range(self):
        import pytest
        from pydantic import ValidationError

        with pytest.raises(ValidationError):
            AirportMatch(code=Airport.JFK, name="JFK", match_type="iata_exact", score=101.0)
```

_{Reviews (1): Last reviewed commit: "refactor: harden airport search with Pyd..." | Re-trigger Greptile}

Greptile also left 2 inline comments on this PR.

[github-actions]

Test Results

  4 files  ± 0    4 suites  ±0   43s ⏱️ -3s
219 tests +19  219 ✅ +19  0 💤 ±0  0 ❌ ±0 
876 runs  +76  876 ✅ +76  0 💤 ±0  0 ❌ ±0 

Results for commit f21a67c. ± Comparison against base commit 6337152.

This pull request removes 5 and adds 24 tests. Note that renamed tests count towards both.

tests.core.test_airports.TestSearchAirports ‑ test_airport_name_san_francisco
tests.core.test_airports.TestSearchAirports ‑ test_city_name_london
tests.core.test_airports.TestSearchAirports ‑ test_code_match_highest_priority
tests.core.test_airports.TestSearchAirports ‑ test_iata_prefix
tests.core.test_airports.TestSearchAirports ‑ test_limit

tests.cli.test_airports.TestAirportsCommand ‑ test_city_query_returns_all_airports
tests.cli.test_airports.TestAirportsCommand ‑ test_exact_code_table_output
tests.cli.test_airports.TestAirportsCommand ‑ test_json_output
tests.cli.test_airports.TestAirportsCommand ‑ test_limit_caps_results
tests.cli.test_airports.TestAirportsCommand ‑ test_no_results_exits_with_error
tests.core.test_airports.TestAirportMatch ‑ test_equality
tests.core.test_airports.TestAirportMatch ‑ test_frozen
tests.core.test_airports.TestAirportMatch ‑ test_rejects_invalid_match_type
tests.core.test_airports.TestAirportMatch ‑ test_rejects_score_out_of_range
tests.core.test_airports.TestSearchAirports ‑ test_city_name_london_returns_all_five
…

♻️ This comment has been updated with latest results.

[greptile-apps]

The position-based score formula has no lower-bound guard. 70.0 - (pos * 0.1) becomes negative when pos > 700. While no real IATA airport name is that long today, Pydantic's ge=0.0 constraint on score would raise a ValidationError the moment it happens, turning search_airports into an unexpected exception in the CLI layer (the MCP layer would catch it via _find_airports_impl's try/except). A max() clamp costs nothing and makes the invariant self-documenting.

Suggested change

	pos = airport_name_lower.find(query_lower)
	score = 70.0 - (pos * 0.1) # Earlier matches score higher.
	results.append(AirportMatch(airport.name, airport.value, "name", score))
	score = 70.0 - (pos * 0.1)
	pos = airport_name_lower.find(query_lower)
	score = max(0.0, 70.0 - (pos * 0.1))

Prompt To Fix With AI

This is a comment left during a code review.
Path: fli/core/airports.py
Line: 157-158

Comment:
The position-based score formula has no lower-bound guard. `70.0 - (pos * 0.1)` becomes negative when `pos > 700`. While no real IATA airport name is that long today, Pydantic's `ge=0.0` constraint on `score` would raise a `ValidationError` the moment it happens, turning `search_airports` into an unexpected exception in the CLI layer (the MCP layer would catch it via `_find_airports_impl`'s try/except). A `max()` clamp costs nothing and makes the invariant self-documenting.

```suggestion
            pos = airport_name_lower.find(query_lower)
            score = max(0.0, 70.0 - (pos * 0.1))
```

How can I resolve this? If you propose a fix, please make it concise.

[greptile-apps]

These three TestAirportMatch tests use a try/except Exception: return pattern rather than pytest.raises. The problem is that the pattern passes for any exception — if Pydantic raises (say) an internal ImportError or a TypeError for an unrelated reason, the test silently greens. Using pytest.raises also pins the expected exception type and gives a cleaner failure message.

Suggested change

	def test_frozen(self):
	m = AirportMatch(code=Airport.JFK, name="JFK Airport", match_type="iata_exact", score=100.0)
	try:
	m.score = 0.0
	except Exception:
	return
	raise AssertionError("AirportMatch should be frozen")
	def test_equality(self):
	a = AirportMatch(code=Airport.JFK, name="JFK", match_type="city", score=90.0)
	b = AirportMatch(code=Airport.JFK, name="JFK", match_type="city", score=90.0)
	assert a == b
	def test_rejects_invalid_match_type(self):
	try:
	AirportMatch(code=Airport.JFK, name="JFK", match_type="bogus", score=50.0) # type: ignore[arg-type]
	except Exception:
	return
	raise AssertionError("AirportMatch should reject unknown match_type")
	def test_rejects_score_out_of_range(self):
	try:
	AirportMatch(code=Airport.JFK, name="JFK", match_type="iata_exact", score=101.0)
	except Exception:
	return
	raise AssertionError("AirportMatch should reject score > 100")
	def test_frozen(self):
	import pytest
	from pydantic import ValidationError
	m = AirportMatch(code=Airport.JFK, name="JFK Airport", match_type="iata_exact", score=100.0)
	with pytest.raises(Exception):
	m.score = 0.0
	def test_equality(self):
	a = AirportMatch(code=Airport.JFK, name="JFK", match_type="city", score=90.0)
	b = AirportMatch(code=Airport.JFK, name="JFK", match_type="city", score=90.0)
	assert a == b
	def test_rejects_invalid_match_type(self):
	import pytest
	from pydantic import ValidationError
	with pytest.raises(ValidationError):
	AirportMatch(code=Airport.JFK, name="JFK", match_type="bogus", score=50.0) # type: ignore[arg-type]
	def test_rejects_score_out_of_range(self):
	import pytest
	from pydantic import ValidationError
	with pytest.raises(ValidationError):
	AirportMatch(code=Airport.JFK, name="JFK", match_type="iata_exact", score=101.0)

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/core/test_airports.py
Line: 116-141

Comment:
These three `TestAirportMatch` tests use a `try/except Exception: return` pattern rather than `pytest.raises`. The problem is that the pattern passes for *any* exception — if Pydantic raises (say) an internal `ImportError` or a `TypeError` for an unrelated reason, the test silently greens. Using `pytest.raises` also pins the expected exception type and gives a cleaner failure message.

```suggestion
    def test_frozen(self):
        import pytest
        from pydantic import ValidationError

        m = AirportMatch(code=Airport.JFK, name="JFK Airport", match_type="iata_exact", score=100.0)
        with pytest.raises(Exception):
            m.score = 0.0

    def test_equality(self):
        a = AirportMatch(code=Airport.JFK, name="JFK", match_type="city", score=90.0)
        b = AirportMatch(code=Airport.JFK, name="JFK", match_type="city", score=90.0)
        assert a == b

    def test_rejects_invalid_match_type(self):
        import pytest
        from pydantic import ValidationError

        with pytest.raises(ValidationError):
            AirportMatch(code=Airport.JFK, name="JFK", match_type="bogus", score=50.0)  # type: ignore[arg-type]

    def test_rejects_score_out_of_range(self):
        import pytest
        from pydantic import ValidationError

        with pytest.raises(ValidationError):
            AirportMatch(code=Airport.JFK, name="JFK", match_type="iata_exact", score=101.0)
```

How can I resolve this? If you propose a fix, please make it concise.