TPN CancelPolicies investigation report — VB→C# migration

cancelpolicies-investigation-report.md

Investigation Report — CancelPolicies handled as a single CancelPenalty instead of a list (TPN connector)

Date: 2026-08-17 Repository: travelgate-connectors/integrations-pull-hotel-tpn Related fix: ITPN-971 / PR #265 ("fix: Correct cancelPolicies")

---

1. Executive summary

The claim under investigation was:

"Since the VB→C# migration, the AI changed the response models and, instead of treating CancelPolicies as a list of CancelPenalty, it only picked up the first policy — always the least restrictive one — which was the only one shown."

The investigation confirms the symptom (only the first, least restrictive CancelPenalty was surfaced) and confirms the fix (ITPN-971, merged 2026-07-29). However, git history shows the root cause was NOT introduced by the AI during the VB→C# migration:

• The defective model — CancelPolicies containing a singular CancelPenalty field instead of a list — already existed in the original VB code, from the very first commit of this repository (1c5a79a "Import project", 2024-06-06).
• The AI-assisted migration (372cf24 "feat: Convert VB to C#", PR #194, merged 2025-12-18) faithfully reproduced the VB model 1:1, singular field included.
• Because .NET XmlSerializer behaves identically in VB.NET and C# (a singular field bound to a repeated XML element keeps the first occurrence — empirically verified during this investigation), the behavior did not change at migration time. The bug was latent in VB and was simply carried over.

The bug was live in production code from at least June 2024 (repo import) until 2026-07-29 (PR #265), not merely from the migration date.

---

2. Root cause (technical)

2.1 The model

TPN's API spec (docs/supplier/spec_5.04.000.md, §1.5) states that a CancelPolicies element contains one CancelPenalty element per cancel rule — i.e., a list:

<CancelPolicies>
    <CancelPenalty>...</CancelPenalty>   <!-- rule 1: e.g. 10% before D-7 -->
    <CancelPenalty>...</CancelPenalty>   <!-- rule 2: e.g. 50% before D-2 -->
    <CancelPenalty>...</CancelPenalty>   <!-- rule 3: e.g. 100% no-show -->
</CancelPolicies>

But the deserialization model (Types/HotelAvailRS.vb, later Types/HotelAvailRS.cs) declared:

' VB — present since initial import 1c5a79a (2024-06-06)
Public Class CancelPolicies
    <XmlElement(ElementName:="CancelPenalty")>
    Public CancelPenalty As CancelPenalty      ' ← singular
End Class

// C# — migrated 1:1 in 372cf24 (2025-11-25, PR #194)
public class CancelPolicies
{
    [XmlElement(ElementName = "CancelPenalty")]
    public CancelPenalty CancelPenalty;        // ← singular, identical to VB
}

2.2 XmlSerializer behavior — empirically verified

A .NET test run during this investigation with 3 repeated <CancelPenalty> elements (AgentPrice 100, 500, 900):

Model field	Result
public CancelPenalty CancelPenalty; (singular)	Keeps first element only (AgentPrice = 100)
public List<CancelPenalty> Pens; (list)	Keeps all 3 elements

TPN returns penalties ordered by application date (earliest/least restrictive first), so the singular field always captured the least restrictive rule — exactly the reported symptom. All downstream consumers (UtilTPN.GetCancelPolicies, UtilACG.GetCancelPoliciesACG, PolicyHours, Quote managers) then had access to only that one penalty.

2.3 What was NOT affected

• ASAT external-rate types were always correct: Types/TypesASAT/HotelAvailRS_ASAT.vb/.cs declared CancelPenalty As List(Of CancelPenalty) / List<CancelPenalty>, and Types/CancelPoliciesASAT.vb/.cs declared List(Of CancelPolicy) — in both VB and C#.
• CancelPenaltyABT (ABT flow) was already a list in both languages.
• CancelPenaltyServiceLineId (a different concept — the service line created for a charged penalty) was never involved.

---

3. Timeline

Date	Commit / PR	Event
2024-06-06	1c5a79a "Import project"	Initial VB import. CancelPolicies.CancelPenalty already singular. Bug latent from day one of this repo (and presumably older, in the pre-import codebase).
2024–2025	e35e082, ff7e309, d36b307	VB evolutions (ABT flow, GWT adaptation). Singular field unchanged in all revisions.
2025-11-25	372cf24 "feat: Convert VB to C#"	AI migration. Model copied 1:1 — singular field preserved, not introduced. ASAT list types also preserved correctly.
2025-12-18	ba7cbd4 (PR #194 merge)	C# code reaches main. No behavioral change vs VB regarding cancel policies.
2025-12 → 2026-07	~40 commits touch UtilTPN.cs; zero touch Types/HotelAvailRS.cs	Model untouched between migration and fix.
2026-07-29	93cc063 (PR #265, ITPN-971; commits b451af7/40667ee "fix: Set CancelPenalty as list")	Fix: model changed to List<CancelPenalty> CancelPenalties + consumers rewritten to iterate all penalties.

4. The fix (ITPN-971, PR #265) — scope

Files changed: Types/HotelAvailRS.cs, Util/UtilTPN.cs, Util/ACG/UtilACG.cs, Util/Mappers/CancellationPolicy/PolicyHours.cs, Manager/Availability/AvailabilityACG.cs, Manager/Availability/AvailabilityDefault.cs, Manager/ListadoRegimenesAsync.cs.

Key changes:

• CancelPolicies.CancelPenalty (singular) → List<CancelPenalty> CancelPenalties.
• UtilTPN.GetCancelPolicies (Quote/Search mapping): now nested foreach over every CancelPenalties item of every CancelPolicies element, with null/empty guards — one cPoliticaCancelacion per penalty, for all domains (ASAT, RSOL/KAT/GWT, ASA, default).
• UtilACG.GetCancelPoliciesACG: same list-iteration rewrite.
• AvailabilityACG/AvailabilityDefault: fixed a secondary bug — used OptStayResults.FirstOrDefault() instead of the current optStayResult in the loop.

No unit tests were added with the fix.

---

5. Residual findings at current HEAD (5e308e1)

The main list handling is fixed, but some code paths still consider only the first penalty:

#	Location	Detail	Risk
R1	Manager/Availability/AvailabilityDefault.cs:736-738 and AvailabilityACG.cs:384-387	Dynamic-rates branch builds one policy from CancelPolicies?.FirstOrDefault()?.CancelPenalties?.FirstOrDefault()?.DeadLine?.OffsetUnitMultiplier. If the supplier returns multiple rules on Search, only the first is exposed in availability results.	Medium — Search shows one policy; Quote later shows the full set, causing Search/Quote inconsistency.
R2	Util/Mappers/CancellationPolicy/PolicyHours.cs:27,60	HasJustDescription inspects only CancelPenalties[0]; CreatePolicyFromDescription builds hours from CancelPenalties.FirstOrDefault()?.PenaltyDescription only (GWT external-rate description path).	Low–Medium — multi-rule descriptive policies collapse to one.
R3	Util/UtilTPN.cs:3518, 3544-3545 (ReservationRead)	Deadline and refund price read from CancelPolicies?.CancelPenalties?.FirstOrDefault() only. If a booking has several active rules, the reported deadline/price may be wrong (least restrictive).	Medium — affects cancellation-cost display on GetBooking.
R4	Types/ConsultaReservaRS.cs:128-129	conrsService.CancelPolicies is a single CancelPolicies element (not a list). Per spec, GetBooking returns one CancelPolicies container, so this is likely correct — but worth validating against real multi-policy bookings.	Low.
R5	Test data (test/.../TestData/ASA.cs, ASAT.cs, RSOL.cs)	Every sample XML contains exactly one CancelPenalty per CancelPolicies (7/7, 9/9, 4/4). There is no regression test proving multi-penalty responses map to multiple cPoliticaCancelacion.	High (regression risk) — the exact bug could silently reappear.

---

6. Conclusions

1. Symptom confirmed: until 2026-07-29 only the first (least restrictive) CancelPenalty was deserialized and shown, due to a singular model field + XmlSerializer first-occurrence semantics.
2. Attribution corrected: the AI migration did not change the model — the singular field is verbatim in the VB source since the repository's first commit (June 2024). The bug pre-dates the migration; VB behaved identically. The migration merely carried the latent defect forward (and was arguably a missed opportunity to catch it, but it introduced no regression here).
3. Fix verified: ITPN-971 (PR #265) correctly converts the model to a list and iterates all penalties in the Quote/Search mapping paths.
4. Residual risk exists in the Search dynamic-rates branch, the GWT description path, and ReservationRead, which still surface only the first penalty; and there is no multi-penalty unit-test coverage.

7. Recommendations

1. Add regression tests with supplier XML containing ≥2 CancelPenalty per CancelPolicies, asserting that all policies appear in politicasCancelacion (Search and Quote). This is the highest-value follow-up (R5).
2. Review R1 (Search dynamic rates) to map all penalties, not just the first, keeping Search consistent with Quote.
3. Review R3 (ReservationRead) — when multiple rules exist, the applicable one per spec is "the rule with the most recent application date", not the first element.
4. Audit R2 (GWT description-only policies) for multi-rule cases.
5. Consider a lightweight model-vs-spec review checklist for future migrations: any [XmlElement] field whose spec says "one or more" must be List<T>.

---

Appendix — Evidence commands

git show 1c5a79a:Types/HotelAvailRS.vb   | grep -A4 "Class CancelPolicies"   # singular in original VB
git show 372cf24^:Types/HotelAvailRS.vb  | grep -A4 "Class CancelPolicies"   # singular right before migration
git show 372cf24:Types/HotelAvailRS.cs   | grep -A4 "class CancelPolicies"   # singular after migration (1:1 copy)
git diff 36f993e 93cc063 -- Types/HotelAvailRS.cs                            # ITPN-971: singular → List<CancelPenalty>
git log --oneline ba7cbd4..36f993e -- Types/HotelAvailRS.cs                  # (empty) model untouched between migration and fix

XmlSerializer first-occurrence proof: deserializing 3 repeated <CancelPenalty> nodes into a singular field yields the first node's values; into a List<T> field yields all 3 (verified with .NET on 2026-08-17).