Skip to content

fix(briefing): locate FPL field 13 by shape in Python, as Swift does - #29

Merged
roznet merged 1 commit into
mainfrom
claude/epic-franklin-2qvalo
Oct 6, 2026
Merged

roznet merged 1 commit into
mainfrom
claude/epic-franklin-2qvalo

Conversation

@roznet

@roznet roznet commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Ports the Swift field-13 shape match (ICAOFlightPlanParser.swift) to parse_icao_fpl, fixing the split 9/10 + no 18/19 misparse.

Closes #26

Owner's brief: #26 Python FPL parser: find field 13 by shape

Kind: 100% parser behaviour (Python only, no API change)
In one line: parse_icao_fpl now finds field 13 by its ICAO+HHMM shape, the same regex and rule as ICAOFlightPlanParser.swift, so an FPL with fields 9 and 10 split and no field 18/19 no longer shifts departure/route/destination by one slot.

What changes for consumers

  • flyfun-weather: plans in that shape (e.g. -C172/L -S/C -EGKA0900 ... with nothing after field 16) now parse correctly instead of departure "S/C". All other well-formed plans give the same output as before. Gets it once it bumps its euro-aip pin.
  • flyfun-apps: none (not listed as a parse_icao_fpl caller in consumers.md).
  • Swift apps: none, no Swift change.

Decisions I made for you

  • Exact port of Swift, including the regex validating the time (([0-1]\d|2[0-3])[0-5]\d), not the looser [A-Z]{4}\d{4} the issue text quotes. Parity over the issue's wording. Easy to flip.
  • Kept Swift's quirk that the scan starts at field 9 itself (index 2). Harmless (field 9 can't match) but kept for identical output.

How it could go wrong

  • Malformed field 13 (bad time, e.g. -EGLL99) plus a field 16 with no alternate (-EGSS0025): field 16 has field 13's shape and is taken as departure. Old Python code would have used slot 3. Same behaviour as Swift today; recorded as a gotcha in the design doc. You'd notice it as departure == destination-like airport and no route.

How we know it works

  • Verified: test_icao_fpl.py 68 passed (12 new); the split-9/10 test fails on the old code and passes now. Full suite: 1057 passed, 3 failed, all 3 also failing on origin/main (test_autorouter_notam coordinates, METAR flight-category IFR, aip_parser parse_airports).
  • Swift test cases (sample, IFR, minimal, metric, field 19, autorouter single line, ZZZZ) ported as a parametrized test with the same departure/destination/time/level/EET/waypoint expectations.
  • Not verified: no Swift run needed (no Swift change); no real-world FPL beyond the repo samples.

Worth making sure you understand

  • Why does a split field 10 now get parsed only when field 13 is found at index 4 or later, and could anything other than field 10 sit in slot 3?
  • Should both parsers restrict the scan to slots 3 and 4 (the only places field 13 can be) to close the malformed-field-13 gotcha? That's a joint Swift+Python change.

Parity: Python now matches Swift; Swift design doc notes Python uses the same regex.
Design docs: briefing_models.md "Locating Field 13" rewritten (dropped "Python still uses the count", added the field-16 gotcha); one line in swift_briefing.md Gotchas.
Release: patch, euro_aip 0.19.1 → 0.19.2 (pip only). No DB rebuild, no SPM tag.
Consumer follow-up: flyfun-weather: bump pin to >=0.19.2 after release.

Pick up on a Mac: nothing (Python-only).

Follow-ups noticed (not done): restrict the field-13 scan to slots 3..4 on both sides (above); 3 pre-existing test failures on main.

🤖 Generated with Claude Code

https://claude.ai/code/session_017wHeKu6ZBSduENF1cjtNTe


Generated by Claude Code

parse_icao_fpl counted fields to find field 13. With fields 9 and 10
split and no field 18/19 (7 fields), it read field 10 ("S/C") as field
13, shifting departure, route and destination by one slot while still
returning a plan. Port the Swift rule from ICAOFlightPlanParser.swift:
first field from field 9 on matching ICAO+HHMM (ZZZZ included), any slot
between field 9 and it is field 10, fall back to index 3.

Tests cover the split 9/10 case, split + field 19, joined vs split
agreement, ZZZZ, and the Swift test cases' departure/route/destination.

Closes #26

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017wHeKu6ZBSduENF1cjtNTe
@claude

claude Bot commented Oct 6, 2026

Copy link
Copy Markdown

Code Review

Reviewed the Python port of Swift's field-13 shape match in parse_icao_fpl, its tests and the two design-doc edits. The regex and fallback match ICAOFlightPlanParser.swift. fields[field13_idx] is safe because of the existing len(fields) >= 6 guard. The split-9/10 case is covered by tests. The malformed-field-13 gotcha is documented and matches Swift. This is a Python-only fix with no API change, and the version bump is left to /release, as the project rules require. No issues found; approving.

@roznet
roznet merged commit 278e96c into main Oct 6, 2026
2 checks passed
@roznet
roznet deleted the claude/epic-franklin-2qvalo branch October 6, 2026 01:49
roznet added a commit that referenced this pull request Oct 6, 2026
a495e8c made _row_to_notam read the API's Garmin semicircles, but the test
rows still carried decimal degrees, so test_coordinates had failed on main
since. The full suite is green again (1071 passed).

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

Python FPL parser: find field 13 by shape, as Swift does (split 9/10 with no 18/19 misparses)

2 participants