feat: add --return-time / return_departure_window for separate return leg time filter#196
Open
RobertoReale wants to merge 6 commits into
Open
feat: add --return-time / return_departure_window for separate return leg time filter#196RobertoReale wants to merge 6 commits into
RobertoReale wants to merge 6 commits into
Conversation
Add explicit error when --min-duration exceeds --max-duration instead of silently returning empty results. Also add min=1 constraint on both CLI options to prevent zero/negative durations from reaching the API.
trip_type was only assigned inside the for loop body, so Pylance flagged it as possibly unbound. Since its value depends only on is_round_trip (constant per call), initialize it once before the loop.
… leg time filter Adds a new optional parameter to both CLI and MCP that lets callers specify an independent departure time window for the return leg of a round-trip search, decoupled from --time / departure_window which controls the outbound leg. - builders.py: return_time_restrictions kwarg on build_flight_segments and build_date_search_segments; False (default) = inherit outbound, None = no filter, TimeRestrictions = explicit override - CLI (flights + dates): --return-time / -T option - MCP (search_flights, search_dates, get_booking_options): return_departure_window field
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+303
to
+304
| if (min_duration is not None or max_duration is not None) and not is_round_trip: | ||
| raise ValueError("--min-duration and --max-duration require --round") |
Contributor
There was a problem hiding this comment.
--return-time silently ignored on one-way date search
--min-duration and --max-duration without --round immediately raise a ValueError with a clear error message, but --return-time without --round is accepted without complaint — the restriction is parsed into return_time_restrictions and then passed to build_date_search_segments, which simply never appends a return segment for one-way trips, silently discarding the filter. A user who forgets --round while using --return-time will receive results without the requested return window applied and no indication that anything is wrong.
Prompt To Fix With AI
This is a comment left during a code review.
Path: fli/cli/commands/dates.py
Line: 303-304
Comment:
**`--return-time` silently ignored on one-way date search**
`--min-duration` and `--max-duration` without `--round` immediately raise a `ValueError` with a clear error message, but `--return-time` without `--round` is accepted without complaint — the restriction is parsed into `return_time_restrictions` and then passed to `build_date_search_segments`, which simply never appends a return segment for one-way trips, silently discarding the filter. A user who forgets `--round` while using `--return-time` will receive results without the requested return window applied and no indication that anything is wrong.
How can I resolve this? If you propose a fix, please make it concise.
Comment on lines
+386
to
+400
| if min_duration is not None or max_duration is not None: | ||
| actual_min = min_duration if min_duration is not None else 1 | ||
| if max_duration is not None: | ||
| actual_max = max_duration | ||
| else: | ||
| from_dt = datetime.strptime(start_date, "%Y-%m-%d") | ||
| to_dt = datetime.strptime(end_date, "%Y-%m-%d") | ||
| actual_max = max(actual_min, (to_dt - from_dt).days) | ||
| if actual_min > actual_max: | ||
| raise ValueError( | ||
| f"--min-duration ({actual_min}) must not exceed --max-duration ({actual_max})" | ||
| ) | ||
| durations_to_search = list(range(actual_min, actual_max + 1)) | ||
| else: | ||
| durations_to_search = [trip_duration] if trip_type == TripType.ROUND_TRIP else [None] |
Contributor
There was a problem hiding this comment.
--max-duration is omitted
When only --min-duration is supplied, actual_max is derived from the entire date-range span: (to_dt - from_dt).days. With a default 60-day window and --min-duration 1, this produces 60 iterations each separated by time.sleep(1) — roughly a 60-second stall with no progress feedback. The same pattern exists in _execute_date_search in server.py. Consider capping the derived maximum (e.g., at 30 days) or requiring --max-duration whenever --min-duration is used.
Prompt To Fix With AI
This is a comment left during a code review.
Path: fli/cli/commands/dates.py
Line: 386-400
Comment:
**Unbounded iteration count when `--max-duration` is omitted**
When only `--min-duration` is supplied, `actual_max` is derived from the entire date-range span: `(to_dt - from_dt).days`. With a default 60-day window and `--min-duration 1`, this produces 60 iterations each separated by `time.sleep(1)` — roughly a 60-second stall with no progress feedback. The same pattern exists in `_execute_date_search` in `server.py`. Consider capping the derived maximum (e.g., at 30 days) or requiring `--max-duration` whenever `--min-duration` is used.
How can I resolve this? If you propose a fix, please make it concise.- --return-time without --round now raises an explicit error, consistent with --min-duration behaviour - --min-duration without --max-duration now raises an error instead of silently deriving an unbounded iteration count from the date range - Add tests for both new validation paths
Author
|
All feedback from the Greptile review has been addressed in subsequent commits. Happy to adjust anything if needed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
--time/departure_windowapplies the same time window to both legs of a round trip. This is fine when you want symmetric constraints (e.g. always fly during the day), but breaks the common case where you want a morning outbound and an evening return -- or vice versa. Without this flag you have to post-filter results manually.What this adds
--return-time/-Tonfli flightsandfli dates, andreturn_departure_windowon the MCP toolssearch_flights,search_dates, andget_booking_options. When provided on a round-trip search, the return leg uses its own time window independently of the outbound window.Design
The core change is a three-state sentinel in
builders.py:False(default)NoneTimeRestrictions(...)This is a backwards-compatible default: existing callers that do not pass
--return-timeget exactly the same behaviour as before.Validation
--return-timewithout--roundraises an explicit error (consistent with--min-duration/--max-duration)--min-durationrequires--max-duration(unbounded iteration is prevented)--min-durationmust not exceed--max-durationFiles changed
fli/core/builders.pyreturn_time_restrictionssentinel parameter on both builder functionsfli/cli/commands/flights.py--return-time/-Toption, threaded through_search_flights_corefli/cli/commands/dates.py--return-time/-Toption, validation guard for one-way searchesfli/mcp/server.pyreturn_departure_windowonFlightSearchParamsandDateSearchParamsREADME.md--return-timeandreturn_departure_windowadded to all reference tablestests/core/test_builders.pytests/cli/test_flights.pytests/cli/test_dates.py--return-timeaccepted on round-trip dates searchtests/mcp/test_mcp_server_unit.pytrip_durationconflictTest plan
uv run pytest -vv --ignore=tests/search-- all 448 unit tests passuv run ruff check .-- cleanfli flights BGY NAP 2026-07-22 --round --return 2026-07-27 --time 6-16 --return-time 16-23-- returns only flights with morning outbound and evening returnfli flights BGY NAP 2026-07-22 --time 6-16 --return-time 16-23(no--round) -- exits with clear errorfli dates BGY NAP --round --min-duration 5 --max-duration 5 --return-time 16-23-- applies return window to all iterationsfli dates BGY NAP --min-duration 5 --max-duration 5 --return-time 16-23(no--round) -- exits with clear error