Conversation
…onstop-first Parser: resolve city/airport names to IATA so "ops Lisbon", "price London Dubai", "ops Abu Dhabi" all work. extractAirports handles 1-word and 2-word city names. Price results: show up to 5 options sorted nonstop-first then by price, with dep/arr times and duration per row. TravelPayouts fallback also shows top 5 sorted. Date label shown next to route header so users know what day is being quoted. Suggestions and placeholder updated with natural language examples.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR extends Key changes:
Confidence Score: 4/5Safe to merge after fixing the sort/slice ordering in the TravelPayouts fallback path; the city-name resolution and GF-path changes are correct. One P1 logic defect: the TravelPayouts fallback sorts after slicing, silently hiding nonstop options priced outside the top-5 by price. This directly contradicts the PR's stated nonstop-first guarantee for that code path. The two P2 items (pluralisation, unescaped time strings) are cosmetic/low-risk. src/components/AviationCommandBar.ts — specifically the Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[User input] --> B[parseIntent]
B --> C{OPS / STATUS?}
B --> D{PRICE/PRICES?}
B --> E{FLIGHT/FLT?}
B --> F{NEWS/BRIEF?}
C --> G[extractAirports words.slice 1]
D --> H[filter keywords + dates\nextractAirports nonKeywords]
G --> I[resolveIata per token\ncity match via MONITORED_AIRPORTS]
H --> I
I --> J[OPS Intent]
I --> K[PRICE_WATCH Intent]
K --> L[executeIntent]
L --> M{fetchGoogleFlights}
M -- flights found --> N[sort by stops then price\nslice 0..5\nrender rows ✓]
M -- no flights --> O[fetchFlightPrices fallback]
O --> P[slice 0..5\nthen sort ⚠️ wrong order]
P --> Q[render rows]
Reviews (1): Last reviewed commit: "feat(aviation): city name search, multi-..." | Re-trigger Greptile |
src/components/AviationCommandBar.ts
Outdated
| const rows = quotes.slice(0, 5).sort((a, b) => a.stops !== b.stops ? a.stops - b.stops : a.priceAmount - b.priceAmount).map(q => { | ||
| const stopColor = q.stops === 0 ? '#22c55e' : '#9ca3af'; | ||
| const stopLabel = q.stops === 0 ? 'nonstop' : `${q.stops} stop`; | ||
| return `<div class="cmd-row" style="padding:5px 0;border-bottom:1px solid rgba(255,255,255,.05)"> | ||
| <div style="flex:1">${escapeHtml(q.carrierName || q.carrierIata)}<span style="color:${stopColor};font-size:11px;margin-left:6px">${stopLabel}</span></div> | ||
| <div style="color:#60a5fa;font-weight:600">$${Math.round(q.priceAmount)}</div> | ||
| </div>`; | ||
| }).join(''); |
There was a problem hiding this comment.
Sort applied after slice in fallback path
.slice(0, 5) is called before .sort(...) in the TravelPayouts fallback path. The API returns quotes sorted cheapest-first, so you're first picking the 5 cheapest fares and then re-ordering them within that subset. This means a nonstop option priced outside the cheapest 5 is silently discarded before the nonstop-first sort ever runs — directly undermining the stated feature goal ("sorted nonstop-first").
The Google Flights path above does it correctly (sort → slice). The same pattern should be used here:
| const rows = quotes.slice(0, 5).sort((a, b) => a.stops !== b.stops ? a.stops - b.stops : a.priceAmount - b.priceAmount).map(q => { | |
| const stopColor = q.stops === 0 ? '#22c55e' : '#9ca3af'; | |
| const stopLabel = q.stops === 0 ? 'nonstop' : `${q.stops} stop`; | |
| return `<div class="cmd-row" style="padding:5px 0;border-bottom:1px solid rgba(255,255,255,.05)"> | |
| <div style="flex:1">${escapeHtml(q.carrierName || q.carrierIata)}<span style="color:${stopColor};font-size:11px;margin-left:6px">${stopLabel}</span></div> | |
| <div style="color:#60a5fa;font-weight:600">$${Math.round(q.priceAmount)}</div> | |
| </div>`; | |
| }).join(''); | |
| const rows = quotes.sort((a, b) => a.stops !== b.stops ? a.stops - b.stops : a.priceAmount - b.priceAmount).slice(0, 5).map(q => { |
src/components/AviationCommandBar.ts
Outdated
| const depTime = leg?.departureDatetime?.slice(11, 16) ?? ''; | ||
| const arrTime = f.legs[f.legs.length - 1]?.arrivalDatetime?.slice(11, 16) ?? ''; | ||
| const stopColor = f.stops === 0 ? '#22c55e' : '#9ca3af'; | ||
| const stopLabel = f.stops === 0 ? 'nonstop' : `${f.stops} stop`; |
There was a problem hiding this comment.
Missing pluralization for stop count
${f.stops} stop renders "2 stop", "3 stop" for multi-stop itineraries. The same issue exists in the TravelPayouts path at line 181.
| const stopLabel = f.stops === 0 ? 'nonstop' : `${f.stops} stop`; | |
| const stopLabel = f.stops === 0 ? 'nonstop' : `${f.stops} stop${f.stops > 1 ? 's' : ''}`; |
And line 181:
const stopLabel = q.stops === 0 ? 'nonstop' : `${q.stops} stop${q.stops > 1 ? 's' : ''}`;
| const depTime = leg?.departureDatetime?.slice(11, 16) ?? ''; | ||
| const arrTime = f.legs[f.legs.length - 1]?.arrivalDatetime?.slice(11, 16) ?? ''; |
There was a problem hiding this comment.
Unescaped time strings rendered into HTML
depTime and arrTime are sliced directly from API-provided datetime strings and inserted into the HTML template without escapeHtml. In practice the slice always yields HH:MM (digits + colon), so the risk is very low, but it's inconsistent with how other API-sourced values are handled throughout this file.
Consider wrapping both in escapeHtml(...) when they are interpolated into the template:
${escapeHtml(depTime)}${arrTime ? `–${escapeHtml(arrTime)}` : ''} · ${escapeHtml(fmtDur(f.durationMinutes))}
…int to prevent duplicates
…i-word name duplicates
Summary
ops Lisbon,ops Dubai,ops Abu Dhabi,price London Dubaiall resolve correctly viaMONITORED_AIRPORTS. Two-word city names (Abu Dhabi, New York, etc.) are handled.ops Dubai,price London Dubai,flight EK3).Test plan
ops Lisbonresolves to LIS and returns ops dataops Abu Dhabiresolves to AUHprice London Dubairesolves LHR/DXB and shows multiple resultsprice BEY DXBreturns nonstop options first, with $-sorted fallbackprice LHR DXB 2026-04-15uses explicit date