Skip to content

Feature/INT-1702 - Airline and accommodation sub-tree model alignment - #246

Merged
david-ruiz-cko merged 4 commits into
mainfrom
feature/INT-1702
Sep 29, 2026
Merged

david-ruiz-cko merged 4 commits into
mainfrom
feature/INT-1702

Conversation

@david-ruiz-cko

Copy link
Copy Markdown
Contributor

This pull request significantly improves the structure, documentation, and correctness of the payment processing models in the SDK, particularly around airline and accommodation data. It introduces new classes for airline and accommodation details, corrects data shapes to match the API specification, and adds comprehensive docstrings and field-level comments for clarity.

Improvements to Airline and Accommodation Data Models:

  • Introduced new classes (Ticket, Passenger, PassengerAddress, FlightLegDetails, AirlineData, AccommodationPhone, AccommodationAddress, AccommodationGuest, AccommodationRoom, AccommodationData) with detailed docstrings and field-level comments to accurately represent the API schema and improve code readability. (F17f0249L532R532)
  • Corrected the data shape for airline and passenger fields in both PaymentContextsAirlineData and ProcessingSettings to use single objects instead of lists, matching the API's expected input and preventing request errors. (checkout_sdk/payments/contexts/contexts.pyL3-R165, F17f0249L532R532)

Enhancements to Processing Settings:

  • Expanded the ProcessingSettings class with detailed docstrings and field-level comments, and added missing fields to ensure full coverage of the current API specification. (F17f0249L532R532)
  • Clarified handling of deprecated and backwards-compatible fields, especially around SenderInformation, to guide developers and prevent misuse. (checkout_sdk/payments/payments.pyL515-R520, F17f0249L532R532)

General Documentation and Consistency:

These changes ensure the SDK models are in sync with the latest API requirements, reduce integration errors, and provide clearer guidance for developers working with complex payment data.

@david-ruiz-cko
david-ruiz-cko requested a review from a team September 28, 2026 13:21
@agent-wall-e

agent-wall-e Bot commented Sep 28, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

  • exceeds_bounded_scope:517>250

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 6


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Sep 28, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
exceeds_bounded_scope — 517>250 classifying §2.1 M8 More than 250 non-test, non-doc, non-lockfile lines changed.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@agent-wall-e

agent-wall-e Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

🟠 Advisory review: Concerns worth a look

This PR needs a human approval. Before you give it, these are the things I'd want resolved.

The PR corrects airline/accommodation data shapes (single object vs. list), adds new model classes, improves documentation, and adds serialization tests. The core intent looks correct, but there are several concrete problems a reviewer should resolve.

Concerns

  • PassengerAddress is imported into contexts.py from payments.payments, making PaymentContextsPassenger depend on a class from a different module than its siblings — but this is intentional per the comment. However, the Union import added to payments.py is unused in the visible diff (AirlineData.passenger is annotated as a comment, not a typed Union), so it may be dead import or the annotation is incomplete.
  • The test test_annotations_declare_float_not_int calls typing.get_type_hints(ProcessingSettings) but the visible diff only shows the type annotation for shipping_amount in PaymentContextsProcessing changed to float — the truncated payments.py diff does not confirm that tax_amount, discount_amount, shipping_tax_amount, duty_amount, and original_order_amount on ProcessingSettings were actually changed to float, so the test may assert something that was never changed and will fail or pass vacuously.
  • The integration test test_create_payment_contexts_with_airline_data passes Country.GB (an enum) for passenger.address.country, but PassengerAddress.country is annotated as str; if the serializer reflects the raw enum value rather than its string representation, this could send an unexpected payload to the API.
  • The test test_several_passengers_serialize_as_a_list in airline_data_serialization_test.py validates that a list of passengers serializes correctly, but the accompanying documentation says POST /payment-contexts rejects this form — the test exists without a counterpart integration test that confirms the list form is rejected on the contexts surface, leaving the documented API contract unverified by tests.
  • The comment on AirlineData.passenger references sandbox verification dates in 2026 ('2026-09-25', '2026-09-28'), which appear to be future dates relative to any plausible authoring time, suggesting copy-paste errors or incorrect dates that undermine the credibility of the verification claims.
  • The truncated diff means the full ProcessingSettings class changes in payments.py (the new fields, float annotations, accommodation classes) are not visible, so correctness of those additions cannot be confirmed from what is shown.
  • The List import added to payments.py is not visibly used in any annotation in the shown diff (annotations appear to use plain list lowercase or comments), which may indicate unused imports or incomplete annotation upgrades.

⚠️ The diff was too large to read in full, so this review covers only part of the change.


This is not an approval. wall-e cannot auto-approve this PR — it is an opinion to help whoever does. Advisory review · us.anthropic.claude-sonnet-4-6 · wall-e 2026.06.19-02

@agent-wall-e

agent-wall-e Bot commented Sep 29, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

  • exceeds_bounded_scope:541>250

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 7


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Sep 29, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
exceeds_bounded_scope — 541>250 classifying §2.1 M8 More than 250 non-test, non-doc, non-lockfile lines changed.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@agent-wall-e

agent-wall-e Bot commented Sep 29, 2026

Copy link
Copy Markdown

🟡 Risk Classification: MINOR

Approval route: AI Review + Human Approval
Rollback controls: Staged rollout + rollback

Classification reasons

  • exceeds_bounded_scope:551>250

Operational gates

  • ✅ jira_ticket (INT-1702)
  • ✅ independent_review

Files analysed: 7


wall-e 2026.06.19-02 · policy 6b4ce2b3b45a…

@agent-wall-e

agent-wall-e Bot commented Sep 29, 2026

Copy link
Copy Markdown
🔬 Debug — why this classification?

Each reason code emitted by the classifier, its source clause in the AI in SDLC Control Framework, and what it means.

Reason code Kind Clause Meaning
exceeds_bounded_scope — 551>250 classifying §2.1 M8 More than 250 non-test, non-doc, non-lockfile lines changed.

Kinds:

  • classifying — this rule contributed to the chosen tier.
  • informational — context only; did not by itself decide the tier.

See issue #3 for the proposal to formalise this map as Appendix A of the standards doc.

wall-e 2026.06.19-02 · debug

@sonarqubecloud

Copy link
Copy Markdown

@david-ruiz-cko
david-ruiz-cko merged commit 233ffe5 into main Sep 29, 2026
4 checks passed
@david-ruiz-cko
david-ruiz-cko deleted the feature/INT-1702 branch September 29, 2026 15:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants