jacobh2/F-r-ldraledigplannerare
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix flexible-wish double-booking, extend calendar cache to 2055, add journey tests

Browse Source 
Second real bug found by driving the running app while the suites were green.

Flexible wishes were funded twice across propose -> apply -> re-propose. Because
planned_leave_day carries no wish_id, the engine could not tell that applied days
were that wish's own prior occurrence; it treated them as occupied space, routed
the candidate search around them, and funded the year again on shifted dates while
reporting weeks_funded 1.0. Applying twice turned "one week each spring" into 18
paid days instead of 9. Fixed wishes were unaffected and stay unaffected.

Fix (allocation_service.py), per the approved approach of counting already-applied
days inside the bounding range rather than adding a wish_id column:
- _Ledger._persisted_lookup snapshots existing days BEFORE this run folds in its
  own freshly-computed windows, so a fixed wish's new placement is never mistaken
  for a flexible wish's history
- _reuse_already_funded_window finds this wish's persisted dates within its bound
  and reuses the window containing them, via the ordinary fund_window path
- full funding therefore yields zero new days and partial funding tops up exactly
  the remainder, with no duplication and no false shortfall
- with no persisted dates, or if the reused window is no longer collision-free, it
  falls through to the original search, so an unsatisfied year is never frozen

Calendar cache extended 2026-2035 -> 2026-2055 (39KB -> 116KB) and the allocator
switched to strict, so an uncached year raises UncachedYearError -> ValidationError
-> HTTP 422 instead of silently treating Julafton, Nyarsafton and Midsommarafton as
working days. Also fixed normalize_day dropping the second name when two holidays
share a date (Nationaldagen was being lost). 2049 and 2055 legitimately have 17 red
days, not 18: Pingstdagen coincides with Nationaldagen.

New backend/tests/integration/test_user_journey.py drives the real app over HTTP
across seven journeys: the double-booking guard, the fixed-wish control, apply
idempotency, fixed-never-displaced-by-flexible, flexible wishes tracking Easter
per year, age-4 arithmetic self-consistency, and a recursive check that no Swedish
text reaches the wire now that everything is message_key + data.

New backend/scripts/seed_demo.py seeds a realistic workspace through the real
service layer for manual testing: demo@example.com / DemoPass123!

Backend 414 -> 435 tests. Frontend unchanged at 705.

Still not verified in a browser.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RXMhc9nT6sNz1SL4j8dznL
This commit is contained in:
Jacob Hagstedt
2026-09-14 07:38:07 +02:00
parent 28c632ca36
commit 249e277696
15 changed files with 4828 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/app/services/allocation_service.py
+156 -5
View File
@@ -35,8 +35,13 @@ from app.repositories.personal_data_repository import PersonalDataRepository
from app.repositories.planned_leave_repository import PlannedLeaveRepository
from app.repositories.vacation_days_repository import VacationDaysRepository
from app.repositories.workspace_repository import WorkspaceRepository
from app.utils.errors import NotFoundError
from app.utils.holidays import SUNDAY_HOLIDAY_NAME, get_swedish_holidays
from app.utils.errors import NotFoundError, ValidationError
from app.utils.holidays import (
SUNDAY_HOLIDAY_NAME,
UncachedYearError,
cached_year_bounds,
get_swedish_holidays,
)
from app.utils.tip_predicates import days_left_at_age_4_projection, has_vacation_near_expiry
# ---------------------------------------------------------------------------
@@ -585,6 +590,18 @@ class _Ledger:
(p.parent_personal_data_uuid, p.child_id, p.date): p.day_type
for p in alloc_input.existing_planned
}
# §fix (double-booking on flexible re-propose) — an immutable snapshot
# of ONLY truly-persisted planned days, taken before `record_days()`
# ever folds in this run's freshly-computed (not-yet-applied) windows.
# `_existing_lookup` above is mutated by `record_days()` as pass 1/2
# proceed, so a fixed wish's just-computed placement would otherwise
# be indistinguishable from a flexible wish's own prior occurrence
# (see `TestFixedWishNeverDisplacedByFlexible`, which must still see
# an empty view here). `_reuse_already_funded_window` is the only
# reader.
self._persisted_lookup: dict[tuple[UUID, UUID | None, date], str] = dict(
self._existing_lookup
)
self._reserve_preschool_days()
@@ -1166,18 +1183,118 @@ class _Ledger:
self._existing_lookup[(d.parent_personal_data_uuid, d.child_id, d.date)] = d.day_type
def _candidate_occupied(
self, start: date, end: date, parents: list[ParentFacts], child_id: UUID | None
self,
start: date,
end: date,
parents: list[ParentFacts],
child_id: UUID | None,
exclude_dates: frozenset[date] = frozenset(),
) -> bool:
"""§10.2 step 5 — whether `[start, end]` collides with an
already-claimed date for any of `parents` (paired with `child_id`).
`exclude_dates` (§fix) lets a caller that already knows some of
those claimed dates are this very wish's own prior occurrence treat
them as expected, not a collision.
"""
for d in _dates_in(start, end):
if d in exclude_dates:
continue
for p in parents:
if (p.uuid, child_id, d) in self._existing_lookup:
return True
return False
def _persisted_dates_for_wish(
self,
responsible: _Responsible,
child_id: UUID | None,
bound_start: date,
bound_end: date,
) -> list[date]:
"""§fix — dates already persisted *before this `propose()` run*
(i.e. from a prior apply, via `_persisted_lookup` — never the
mutable `_existing_lookup`) for this wish's resolved parent(s) and
child, inside its bound. The read-only source is what keeps a fixed
wish's in-session placement from being mistaken for a flexible
wish's own history (§10.3, `TestFixedWishNeverDisplacedByFlexible`).
"""
parent_uuids = {p.uuid for p in responsible.parents}
return sorted(
{
d
for (p_uuid, c_id, d) in self._persisted_lookup
if c_id == child_id and p_uuid in parent_uuids and bound_start <= d <= bound_end
}
)
def _reuse_already_funded_window(
self,
wish: LeaveWish,
responsible: _Responsible,
child_id: UUID | None,
bound_start: date,
bound_end: date,
length_days: int,
) -> tuple[date, date] | None:
"""§fix — the double-booking-on-repropose fix, product-owner approved:
if this flexible wish-year already has planned days from a prior
apply inside its bounding range, reuse the full-length window they
belong to instead of searching for a new one. The caller then funds
that window through the ordinary `fund_window()` path exactly like
any other window; because `_fund_solo`/`_fund_swap`/`_fund_both` all
skip emitting a new `ProposedDay` for a date `_existing_lookup`
already has, this single mechanism handles both "fully funded"
(every spendable day in the window is already persisted -> zero new
days) and "partially funded" (only some are -> the remainder is
topped up) without a separate code path for either.
Known and accepted limitation (§10.2's "occupied dates" tradeoff,
now extended): a planned day belonging to a *different* wish that
happens to fall in this same bounding range cannot be told apart
from this wish's own prior occurrence, and may be mistaken for it.
Returns `None` — falls through to a fresh search — when: nothing is
persisted for this wish-year yet, the persisted dates don't fit
inside one `length_days` window (they can't be a single occurrence
of *this* wish), or every window that would contain them now
collides with something else already claimed this run (e.g. a
higher-priority fixed wish added since the last apply). That last
case is what keeps a genuinely-unsatisfied year free to move
(§10.3) rather than frozen to a window that no longer fits.
"""
persisted_dates = self._persisted_dates_for_wish(
responsible, child_id, bound_start, bound_end
)
if not persisted_dates:
return None
span = (persisted_dates[-1] - persisted_dates[0]).days + 1
if span > length_days:
return None
own_dates = frozenset(persisted_dates)
candidates = [
(s, e)
for s, e in _candidate_windows(bound_start, bound_end, length_days)
if s <= persisted_dates[0]
and persisted_dates[-1] <= e
and not self._candidate_occupied(s, e, responsible.parents, child_id, own_dates)
]
if not candidates:
return None
return min(
candidates,
key=lambda c: (
_candidate_cost(c[0], c[1], self._input.holidays, self._input.day_overrides),
c[0],
wish.id.int,
),
)
def resolve_flexible_window(
self, wish: LeaveWish, anchor_year: int, bound_start: date, bound_end: date
) -> _WishWindow | None:
@@ -1201,6 +1318,18 @@ class _Ledger:
resolved_child = self._resolve_wish_child(wish)
child_id = resolved_child.id if resolved_child is not None else None
# §fix — an already (fully or partially) funded wish-year reuses its
# own prior window rather than being searched for fresh, which is
# what caused the double-booking regression.
reused = self._reuse_already_funded_window(
wish, responsible, child_id, bound_start, bound_end, length_days
)
if reused is not None:
winner_start, winner_end = reused
return _WishWindow(
wish=wish, anchor_year=anchor_year, start=winner_start, end=winner_end
)
candidates = [
(s, e)
for s, e in _candidate_windows(bound_start, bound_end, length_days)
@@ -1826,9 +1955,31 @@ class AllocationInputLoader:
is_assumed=True,
)
# `strict=True`: never silently fall back to the live `holidays`
# library for a year outside `swedish_calendar.json`'s cached range —
# that fallback is missing de facto days (Julafton/Nyårsafton/
# Midsommarafton) and previously caused those to be charged as
# working days once a plan's horizon ran past the cache (see
# `app.utils.holidays.UncachedYearError`). With the cache now ~30
# years deep this should never fire for a realistic horizon, but a
# wish with a far-out `last_year` (uncapped when the workspace has no
# children yet) can still in principle reach past it. Rather than
# silently clamping the horizon (which would contradict the §6
# decision to plan past B4 and show the squeeze), surface it as a
# `ValidationError` the router turns into a 422 the UI can explain,
# the same way `CalendarService` handles an out-of-range `start_year`.
holidays_map: dict[date, str] = {}
for year in range(today.year, horizon_end.year + 2):
holidays_map.update(get_swedish_holidays(year))
try:
for year in range(today.year, horizon_end.year + 2):
holidays_map.update(get_swedish_holidays(year, strict=True))
except UncachedYearError as exc:
bounds = cached_year_bounds()
where = f"{bounds[0]}-{bounds[1]}" if bounds is not None else "(cache missing)"
raise ValidationError(
f"Planning horizon ({horizon_end.isoformat()}) extends beyond the cached "
f"Swedish calendar {where}; shorten the wish's last_year or the plan's "
"horizon so it stays within the cached range."
) from exc
day_overrides: dict[date, bool] = {}
if self._day_override_repo is not None: