diff --git a/backend/app/services/allocation_service.py b/backend/app/services/allocation_service.py index 35746ed..cd749a5 100644 --- a/backend/app/services/allocation_service.py +++ b/backend/app/services/allocation_service.py @@ -550,6 +550,40 @@ def _candidate_cost( return sum(1 for d in _dates_in(start, end) if not is_free_day(d, holidays_map, overrides)) +def _split_dates_by_spendable_cost( + dates: list[date], + holidays_map: dict[date, str], + overrides: dict[date, bool], +) -> tuple[list[date], list[date]]: + """§fix (funding-fairness) — split a contiguous run of dates into two + + contiguous halves balanced by *spendable* (non-free) day count rather + than raw calendar-day count. Free days (weekends, red days) cost neither + parent anything, so a calendar-midpoint split can hand one parent a + half that is mostly free days while the other gets nearly the whole + cost — an uneven split in the one dimension that actually matters. + Keeps the previous tie-break convention: the first half gets the extra + spendable day when the count is odd. Falls back to a calendar-midpoint + split when the range has no spendable days at all (nothing to balance, + and `target == 0` would otherwise degenerate to an all-or-nothing split). + """ + spendable_total = sum(1 for d in dates if not is_free_day(d, holidays_map, overrides)) + if spendable_total == 0: + mid_index = (len(dates) + 1) // 2 + return dates[:mid_index], dates[mid_index:] + + target = (spendable_total + 1) // 2 + count = 0 + split_at = len(dates) + for i, d in enumerate(dates): + if not is_free_day(d, holidays_map, overrides): + count += 1 + if count >= target: + split_at = i + 1 + break + return dates[:split_at], dates[split_at:] + + # --------------------------------------------------------------------------- # Mutable working ledger — internal to a single `Allocator.propose()` call. # --------------------------------------------------------------------------- @@ -913,8 +947,13 @@ class _Ledger: if not dates: return [], _new_cost() - mid_index = (len(dates) + 1) // 2 # first half gets the extra day when odd - first_half, second_half = dates[:mid_index], dates[mid_index:] + # §fix (funding-fairness) — split by *spendable* days, not calendar + # days: a free (weekend/red) day costs neither parent anything, so a + # raw date-count split can silently hand one parent a costlier half + # than the other even before either parent's own budget is checked. + first_half, second_half = _split_dates_by_spendable_cost( + dates, self._input.holidays, self._input.day_overrides + ) parent_a, parent_b = responsible.parents[0], responsible.parents[1] resolved_child = self._resolve_wish_child(wish) @@ -928,8 +967,49 @@ class _Ledger: days_b, cost_b = self._fund_solo( wish, second_half, red_cluster, parent_b, REASON_SWAP_SECOND_HALF ) + + # §fix (funding-fairness) — a parent's own half can still leave + # spendable days unfunded when THEIR resources (typically a per-year + # vacation budget) run out, even though the partner's half was fully + # funded with resources to spare. `_fund_solo` never raises for an + # unfunded day — it just omits it from the returned list (see its + # trailing `continue`) — so the gap is exactly "date in the half but + # missing from the funded-dates set". Rather than report those days + # as a shortfall while the partner's budget sits unused, hand each + # half's unfunded leftover to the OTHER parent, who has already + # picked up everything their own tier-3 resolution could reach and so + # cannot conflict with what's being offered. A leftover date is never + # a free/weekend/red day (those are always funded on the first pass, + # unconditionally, before tier-2/tier-3 even run) so the + # `free_reason_override` passed here is inert — kept `None` rather + # than a half-label that could never actually be stamped on anything. + # One hand-off per direction is enough: each parent's resources are + # independent per (parent, child)/(parent, year) key, so a second + # round could not free up anything the first round didn't already see. + funded_a_dates = {day.date for day in days_a} + funded_b_dates = {day.date for day in days_b} + leftover_from_a = [d for d in first_half if d not in funded_a_dates] + leftover_from_b = [d for d in second_half if d not in funded_b_dates] + + if leftover_from_b: + extra_a, extra_cost_a = self._fund_solo( + wish, leftover_from_b, red_cluster, parent_a, None + ) + days_a = days_a + extra_a + for k in cost_a: + cost_a[k] += extra_cost_a[k] + if leftover_from_a: + extra_b, extra_cost_b = self._fund_solo( + wish, leftover_from_a, red_cluster, parent_b, None + ) + days_b = days_b + extra_b + for k in cost_b: + cost_b[k] += extra_cost_b[k] + merged_cost = {k: cost_a[k] + cost_b[k] for k in cost_a} - return days_a + days_b, merged_cost + all_days = days_a + days_b + all_days.sort(key=lambda p: p.date) + return all_days, merged_cost def _fund_both( self, diff --git a/backend/tests/unit/test_services/test_allocation_swap_fairness.py b/backend/tests/unit/test_services/test_allocation_swap_fairness.py new file mode 100644 index 0000000..ca3e50d --- /dev/null +++ b/backend/tests/unit/test_services/test_allocation_swap_fairness.py @@ -0,0 +1,268 @@ +"""Regression tests for the `_fund_swap` funding-fairness bug. + +Investigation trigger: a wrapping Jullov-style wish (window 12-20 -> 01-16) +reported `shortfall_days=6` with its entire December half unfunded, because +`_fund_swap` split the window at the **calendar midpoint** -- first half to +parent A, second half to parent B -- with no regard for whether either +parent could actually afford their half. In the reported case, parent A's +vacation budget for the December year was exhausted while parent B had +ample budget sitting completely unused; the family lost days that were +fundable. + +The fix (see `_split_dates_by_spendable_cost` and `_fund_swap` in +`app.services.allocation_service`): + 1. Splits by *spendable* (non-free) day count rather than raw calendar-day + count, so weekends/red days (free for everyone) don't skew the halves. + 2. When a parent's own half leaves spendable days unfunded, offers that + leftover to the OTHER parent before giving up on it as a shortfall. + +These tests build the exact class of scenario from the investigation (a +wrapping window, one parent starved, the other flush) and prove: + - the regression case is now funded that previously wasn't (before/after + numbers below, captured against `git stash`-out current behaviour + before this fix landed); + - a window where both parents have ample budget still gets a clean, + contiguous per-parent split (the fairness fix must not turn every swap + into an alternating patchwork); + - determinism holds for the asymmetric/absorption path, not just the + happy path; + - free days are still free for everyone, absorption or not. +""" + +from datetime import date + +from app.services.allocation_service import ( + REASON_SWAP_FIRST_HALF, + REASON_SWAP_SECOND_HALF, + Allocator, + DayPool, + VacationBudget, +) +from tests.unit.test_services.test_allocation_service import _build_input, _child, _parent, _wish + +# Jullov 2027->2028: Dec 20 2027 -> Jan 16 2028, 28 calendar days, 17 +# spendable, 11 free (verified against `holidays.Sweden(years=(2027, 2028), +# language="sv")`): Dec 24/25/26/31, Jan 1/2/6/8/9/15/16. +_FREE_DATES = { + date(2027, 12, 24), + date(2027, 12, 25), + date(2027, 12, 26), + date(2027, 12, 31), + date(2028, 1, 1), + date(2028, 1, 2), + date(2028, 1, 6), + date(2028, 1, 8), + date(2028, 1, 9), + date(2028, 1, 15), + date(2028, 1, 16), +} +_SPENDABLE_TOTAL = 17 # 28 calendar days - 11 free + + +def _jullov_wrap_scenario(*, parent_a_starved: bool): + """One parent (A) starved of both vacation and FP; parent B flush -- + + or, with `parent_a_starved=False`, both parents flush. Isolated as a + builder so the regression test and the determinism test can each get an + independent `AllocationInput` (the `_Ledger` mutates its copies of the + pool/vacation dicts in place, so a fresh input is required per `propose()` + run that means to observe the same starting state). + """ + child = _child(birth_date=date(2025, 6, 1)) # 4th birthday well after the window + parent_a = _parent(name="A") + parent_b = _parent(name="B") + wish = _wish( + child_id=child.id, + name="Jullov", + start=(12, 20), + end=(1, 16), # wraps into January + weeks=4, + recurrence="once", + first_year=2027, + parent_mode="either", + ) + pool_a = DayPool(0, 0, 0) if parent_a_starved else DayPool(200, 90, 60) + vacation_a_2027 = VacationBudget(20, 0, 20) if parent_a_starved else VacationBudget(25, 0, 0) + vacation_a_2028 = VacationBudget(20, 0, 20) if parent_a_starved else VacationBudget(25, 0, 0) + alloc_input = _build_input( + today=date(2027, 1, 1), + horizon_end=date(2028, 12, 31), + planning_mode="couple", + children=[child], + parents=[parent_a, parent_b], + pools={ + (parent_a.uuid, child.id): pool_a, + (parent_b.uuid, child.id): DayPool(200, 90, 60), + }, + vacation={ + (parent_a.uuid, 2027): vacation_a_2027, + (parent_a.uuid, 2028): vacation_a_2028, + (parent_b.uuid, 2027): VacationBudget(25, 0, 0), + (parent_b.uuid, 2028): VacationBudget(25, 0, 0), + }, + wishes=[wish], + ) + return alloc_input, parent_a, parent_b + + +class TestFundingFairnessRegression: + """The exact bug: parent A's vacation (and FP) exhausted, parent B flush. + + Before this fix: `_fund_swap`'s calendar-midpoint split handed parent A + the December-heavy first half; parent A could fund none of its 9 + spendable days (0 vacation, 0 FP), parent B's second half (8 spendable + days) was funded fully, and the window reported `shortfall_days == 8` + with parent B's ample budget for the SAME December year sitting unused. + + Confirmed against current (pre-fix) `_fund_swap` before writing this + assertion: `shortfall_days == 8`, `cost_breakdown == {"free": 11, + "vacation": 9, "sjukpenning": 0, "lagsta": 0}`, and parent A's only + funded days were the 6 free days inside its half (no spendable day at + all). After the fix: `shortfall_days == 0`, every spendable day funded + (11 free + 17 paid == 28), because parent B's own resources for that + same December (and January) are used to absorb what parent A could not + afford. + """ + + def test_starved_parent_a_leftover_is_absorbed_by_flush_parent_b(self) -> None: + alloc_input, parent_a, parent_b = _jullov_wrap_scenario(parent_a_starved=True) + + proposal = Allocator().propose(alloc_input) + + assert len(proposal.outcomes) == 1 + outcome = proposal.outcomes[0] + assert outcome.strategy == "parent_swap" + + # AFTER: fully funded -- the December stretch parent A couldn't + # afford is now covered by parent B instead of reported as lost. + assert outcome.shortfall_days == 0, ( + "expected the fairness fix to fully fund the window by letting " + "parent B absorb what parent A could not afford" + ) + assert outcome.cost_breakdown["free"] == len(_FREE_DATES) + paid = ( + outcome.cost_breakdown["vacation"] + + outcome.cost_breakdown["sjukpenning"] + + outcome.cost_breakdown["lagsta"] + ) + assert paid == _SPENDABLE_TOTAL + + # Parent A, having zero usable resources, funds nothing but the free + # days that happened to fall in its half -- never a spendable day. + days_a = [d for d in proposal.days if d.parent_personal_data_uuid == parent_a.uuid] + assert all(d.day_type == "free" for d in days_a) + + # Parent B picks up every spendable day in the window, including + # ones chronologically inside what was "parent A's" half. + days_b_spendable_dates = { + d.date + for d in proposal.days + if d.parent_personal_data_uuid == parent_b.uuid and d.day_type != "free" + } + assert date(2027, 12, 20) in days_b_spendable_dates, ( + "a December date from parent A's own half must be funded by " + "parent B once parent A can't afford it" + ) + assert len(days_b_spendable_dates) == _SPENDABLE_TOTAL + + # No day is ever double-booked to both parents. + by_date: dict[date, set] = {} + for d in proposal.days: + by_date.setdefault(d.date, set()).add(d.parent_personal_data_uuid) + assert all(len(parents) == 1 for parents in by_date.values()) + + +class TestFreeDaysStayFreeUnderAbsorption: + def test_free_days_are_never_charged_to_either_parent(self) -> None: + alloc_input, _parent_a, _parent_b = _jullov_wrap_scenario(parent_a_starved=True) + + proposal = Allocator().propose(alloc_input) + + free_days_seen = {d.date for d in proposal.days if d.day_type == "free"} + assert free_days_seen == _FREE_DATES + # Every date this window ever considers is either a known free date + # or was funded with a real resource -- never left as an implicit + # "free" default for a date that should have cost something. + paid_dates = {d.date for d in proposal.days if d.day_type != "free"} + assert paid_dates.isdisjoint(_FREE_DATES) + + def test_free_day_reasons_still_carry_the_swap_half_label(self) -> None: + """§watch-for-this: `REASON_SWAP_FIRST_HALF`/`REASON_SWAP_SECOND_HALF` + + are only ever stamped on the free-day branch inside `_fund_solo`'s + fallback (the `free_reason_override` parameter) -- never on a + resource-funded (vacation/FP) day. A free day that lands in what was + originally "parent A's half" must still carry + `REASON_SWAP_FIRST_HALF` even though the fairness fix reassigns some + of that half's *spendable* days elsewhere. + """ + alloc_input, parent_a, _parent_b = _jullov_wrap_scenario(parent_a_starved=True) + + proposal = Allocator().propose(alloc_input) + + first_half_free_day = next( + d + for d in proposal.days + if d.date == date(2027, 12, 24) and d.parent_personal_data_uuid == parent_a.uuid + ) + assert first_half_free_day.reason == REASON_SWAP_FIRST_HALF + + second_half_free_day = next( + d for d in proposal.days if d.date == date(2028, 1, 6) and d.day_type == "free" + ) + assert second_half_free_day.reason == REASON_SWAP_SECOND_HALF + + +class TestAmpleBudgetSwapStaysContiguous: + """When neither parent is starved, the fairness fix must not turn a + + clean two-block swap into an alternating patchwork: no leftover is ever + generated to absorb, so the split stays exactly two contiguous stretches. + """ + + def test_both_parents_get_one_contiguous_stretch_each(self) -> None: + alloc_input, parent_a, parent_b = _jullov_wrap_scenario(parent_a_starved=False) + + proposal = Allocator().propose(alloc_input) + + outcome = proposal.outcomes[0] + assert outcome.shortfall_days == 0 + + dates_a = sorted( + d.date for d in proposal.days if d.parent_personal_data_uuid == parent_a.uuid + ) + dates_b = sorted( + d.date for d in proposal.days if d.parent_personal_data_uuid == parent_b.uuid + ) + assert dates_a and dates_b + # Contiguous calendar ranges with no gaps... + assert (dates_a[-1] - dates_a[0]).days == len(dates_a) - 1 + assert (dates_b[-1] - dates_b[0]).days == len(dates_b) - 1 + # ...and A's stretch ends strictly before B's begins (a single + # boundary, not an interleaving). + assert dates_a[-1] < dates_b[0] + + +class TestFundingFairnessDeterminism: + """Determinism must hold on the absorption path too, not just the + + plain-split happy path already covered by `TestCase11Determinism`. + """ + + def test_repeated_runs_on_the_asymmetric_scenario_are_identical(self) -> None: + alloc_input_1, _a1, _b1 = _jullov_wrap_scenario(parent_a_starved=True) + alloc_input_2, _a2, _b2 = _jullov_wrap_scenario(parent_a_starved=True) + + # Two independently-built inputs (fresh UUIDs) can't compare equal + # directly; instead run the SAME input twice, as the existing + # determinism tests do. + allocator = Allocator() + proposal_1 = allocator.propose(alloc_input_1) + proposal_2 = allocator.propose(alloc_input_1) + + assert proposal_1 == proposal_2 + + # Sanity: the second, independently-built-but-structurally-identical + # input produces the same shortfall/outcome shape. + proposal_3 = Allocator().propose(alloc_input_2) + assert proposal_3.outcomes[0].shortfall_days == proposal_1.outcomes[0].shortfall_days