fix: strip underscore prefix from digit-starting codes in all serialization paths#100
Closed
ktrufanov-skycop wants to merge 1 commit intopunitarani:mainfrom
Closed
Conversation
…ization
paths
- Use removeprefix("_") instead of lstrip("_") for safer single-prefix
removal
- Fix date search serialization in dates.py (was still sending _3F to API)
- Fix round-trip selected_flight serialization in flights.py: pass enum
objects to serialize() instead of pre-resolving .name (which bypassed
the prefix-stripping logic)
- Add test for serialize_airline with numeric-prefix code
Contributor
Author
|
Duplicate — resolved conflict in #94 instead. |
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
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.
Summary
Follow-up to #90 (merged). Addresses the serialization gap flagged
by @punitarani in the review comment —
digit-prefixed airline codes like
_3Fwere still leaking into APIrequests and CLI output through paths not covered by the first PR.
Changes
removeprefix("_")inflights.pyanddates.pyserialize()functions — strip the synthetic underscore before sending codes to
Google Flights API
serialize_airline()incli/utils.py— same fix for CLI JSON outputselected_flightinflights.py— pass enum objects(
leg.airline,leg.departure_airport,leg.arrival_airport) toserialize()instead of pre-resolving.name, which bypassed theisinstancecheck and leaked_3Finto the request payloadserialize_airline()with numeric-prefix codeWhy
removeprefixoverlstriplstrip("_")would strip all leading underscores (__3F→3F).removeprefix("_")removes exactly one — matching the single underscoreadded by
generate_enums.pyfor digit-starting IATA codes.Test plan
serialize_airline(Airline._3F)returns{"code": "3F", "name": "FlyOne Armenia"}Greptile Summary
This PR completes the fix for digit-prefixed IATA airline codes (e.g.,
_3F) leaking out of serialization paths, following up on #90. It appliesremoveprefix(\"_\")in the two Google Flights API serializers (FlightSearchFiltersandDateSearchFilters), in the CLI JSON output helper (serialize_airline), and — critically — fixes theselected_flightround-trip block inflights.pythat was passing.namestrings directly toserialize(), bypassing theisinstance(obj, Airline)branch entirely.fli/models/google_flights/flights.py:removeprefix(\"_\")added toserialize(), plusleg.departure_airport,leg.arrival_airport, andleg.airlinenow passed as enum objects (not pre-resolved.namestrings) in theselected_flightsbuilder — this was the root cause of the payload leak.fli/models/google_flights/dates.py: Sameremoveprefix(\"_\")fix applied to the parallelserialize()inDateSearchFilters.format().fli/cli/utils.py:serialize_airline()now strips the underscore from thecodekey in CLI JSON output.tests/cli/test_utils.py: Newtest_serialize_airline_numeric_prefixtest verifiesAirline._3F→{\"code\": \"3F\", \"name\": \"FlyOne Armenia\"}.fli/mcp/server.pyline 294 still useslstrip(\"_\")forairline_code, which differs from theremoveprefix(\"_\")approach used everywhere else after this PR. This is a pre-existing gap not introduced here, but worth aligning.Confidence Score: 5/5
Safe to merge — all serialization paths that send data to the Google Flights API or emit CLI/JSON output are correctly fixed.
All changed code is correct. The critical root-cause fix (passing enum objects instead of pre-resolved
.namestrings in theselected_flightsblock) is sound. The only remaining gap is a pre-existinglstrip("_")in the MCP server that is not introduced by this PR and is P2 only.fli/mcp/server.py line 294 — pre-existing
lstrip("_")inconsistency worth aligning with theremoveprefix("_")approach used everywhere else after this PR.Important Files Changed
removeprefix("_")inserialize()for Airport/Airline enums, and passing enum objects (not.namestrings) toserialize()in theselected_flightsblock — closing the bypass of the isinstance check that leaked_3Finto API payloads.removeprefix("_")fix applied to the parallelserialize()function insideDateSearchFilters.format(); no selected-flight block in this model so no further changes needed.serialize_airline()updated to strip the synthetic underscore from thecodekey in CLI JSON output;serialize_airport()is unchanged (correct, as Airport has no digit-starting codes).test_serialize_airline_numeric_prefixtest verifies thatAirline._3Fround-trips to{"code": "3F", "name": "FlyOne Armenia"}viaserialize_airline().Sequence Diagram
sequenceDiagram participant User participant Serializer participant GoogleFlightsAPI Note over User,GoogleFlightsAPI: Before PR — digit-prefix leak User->>Serializer: leg.airline.name passes string "_3F" Note over Serializer: No isinstance match, falls through Serializer->>GoogleFlightsAPI: payload contains "_3F" (incorrect) Note over User,GoogleFlightsAPI: After PR — correct round-trip User->>Serializer: leg.airline passes Airline enum object Note over Serializer: isinstance(obj, Airline) matches Serializer->>GoogleFlightsAPI: obj.name.removeprefix returns "3F" (correct)Comments Outside Diff (1)
fli/mcp/server.py, line 294 (link)The MCP server still uses
lstrip("_")here, while every other serialization path in this PR (and the PR description itself) explicitly prefersremoveprefix("_")— becauselstrip("_")would strip all leading underscores (e.g., a hypothetical__3F→3F), whereasremoveprefix("_")removes exactly the one underscore added bygenerate_enums.py.For parity with the rest of the codebase after this PR:
Prompt To Fix With AI
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: strip underscore prefix from digit-..." | Re-trigger Greptile
(4/5) You can add custom instructions or style guidelines for the agent here!