Skip to content

fix(weather): metric visibility of 8000 m or more is VFR - #35

Merged
roznet merged 1 commit into
mainfrom
claude/pensive-dijkstra-uqrgni
Oct 11, 2026
Merged

roznet merged 1 commit into
mainfrom
claude/pensive-dijkstra-uqrgni

Conversation

@roznet

@roznet roznet commented Oct 10, 2026

Copy link
Copy Markdown
Owner

For roznet/flyfun-weather#757 (consumer PR: roznet/flyfun-weather, branch claude/pensive-dijkstra-uqrgni).

What

A European report of 8000 m converted to 4.97 SM and landed in the FAA MVFR band (3 to 5 SM). flyfun-weather raised false "TAF at ETA: VFR → MVFR" alerts from TEMPO … 8000 SHRA. 5 SM is 8047 m: only the unit conversion made 8000 m marginal.

  • WeatherReport.visibility_unit ("M" / "SM", None when unknown) is new. The parser sets it, and TAF group application, _worse_of and to_dict/from_dict carry it through.
  • WeatherAnalyzer.flight_category: a metric report with visibility ≥ METRIC_VFR_VISIBILITY_M (8000, exported) is VFR. SM reports keep the FAA edge (5SM MVFR, 6SM VFR). A report with no unit falls back to the FAA rule, as before.
  • This departs from the issue's suggested test. The issue said to treat "has visibility_meters" as metric. That doesn't work because the parser fills metres for SM reports too (5SM → 8046 m), so 5SM would have read VFR. The unit field is the fix for that.

Additive, no breaking API change → minor bump to 0.22.0 via /release. flyfun-weather's PR pins euro-aip>=0.22.0.

No Swift counterpart: RZFlight has no flight-category code (checked).

Verification

  • pytest tests/briefing/test_weather: 337 passed. 13 are new: METAR 8000/9999/7000/5000/4000, 5SM/6SM, no-unit fallback, dict round trip, the EGJB TAF with TEMPO 8000 → VFR, TEMPO 7000 → MVFR.
  • Full euro_aip suite: 1122 passed, 1 failed (test_aip_parser_parse_airports). It fails the same way on the branch without this change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01MDyEuQSS1UNjQU4AViBbxf


Generated by Claude Code

A European report of 8000 m converted to 4.97 SM and landed in the FAA
MVFR band (3-5 SM), so `TEMPO 8000 SHRA` raised false "VFR -> MVFR" TAF
alerts in flyfun-weather. 5 SM is 8047 m; only the conversion made
8000 m marginal.

WeatherReport gains `visibility_unit` ("M" / "SM"), set by the parser and
carried through TAF group application and serialisation. A metric report
with visibility >= METRIC_VFR_VISIBILITY_M (8000) is VFR; SM reports keep
the FAA edge (5SM stays MVFR). The unit is needed because the parser fills
visibility_meters for SM reports too (5SM -> 8046 m). A report without a
unit falls back to the FAA rule.

For roznet/flyfun-weather#757

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

claude Bot commented Oct 10, 2026

Copy link
Copy Markdown

Code Review

Reviewed the metric-visibility VFR fix: new visibility_unit field, parser/TAF overlay/_worse_of propagation, flight_category change, design doc and tests. The change is additive. The unit-based approach is correct, since visibility_meters is filled for SM reports too. The unit propagates through every construction path I found, and there is no Swift counterpart to update. The design doc matches the code, and the version bump is correctly left to /release. No issues found.

@roznet
roznet merged commit 25caea0 into main Oct 11, 2026
2 checks passed
@roznet
roznet deleted the claude/pensive-dijkstra-uqrgni branch October 11, 2026 13:57
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.

2 participants