Parse transferable days from FK instead of deriving them
The scraper never read "överlåtbara dagar" from Försäkringskassan. It only visited ansok-och-planera-foraldrapenning#/valjdagar and derived the reserved/transferable split in build_snapshot, assuming transferable days are spent before reserved ones. On real data that gave 49 for the user and 0 for the partner where FK states 129 and 71. The assumption cannot be repaired: reserved-first, transferable-first, proportional and cap-based models were all checked against real figures and none reproduces FK's accounting (reserved-first fits the partner but is 10 days off for the user). FK publishes the real numbers per parent per tier on the min-foraldrapenning page, which the scraper had never visited. - Scrape the "Dagar ni kan föra över" panel after the valjdagar extraction, summing sjukpenningnivå + lägstanivå per parent. The totals are not literals on the page. Parse only direct-child <p> elements: the first group embeds a tooltip with decoy "45 dagar"/"90 dagar" text. - build_snapshot accepts the reported value and derives reserved days from it; the old arithmetic remains only as a fallback. An implausible FK value degrades to that fallback rather than failing the fetch. - Record provenance in transferable_days_reported (migration 006) and badge derived values as estimates in the dashboard breakdown. Drop the footnote's claim that at most 150 days are transferable — real data contradicts it as a per-family figure. - Select the child's section by name when FK lists several; never guess. Verified against a live BankID scrape: scraped 240/101/139 with 129 transferable, deduced partner 240/169/71 with 71, both flagged as reported. Incidental fixes found along the way: - The fetch router never forwarded the partner's transferable figure to persist_partner_deduction, so the partner's stored snapshot would have kept the flawed split even once the parse landed. - 'deduced' snapshots were labelled "Manually entered" by a two-way ternary over a three-value union. Replaced with a compiler-checked exhaustive map. - Alembic resolved its database from alembic.ini alone, ignoring APP_SQLITE_PATH, so migrations silently targeted the developer's real database even when pointed elsewhere. env.py now resolves it, preferring -x sqlalchemy.url, then a real APP_SQLITE_PATH, then the ini value. - backend/captures/ was not ignored: the root pattern was anchored as /captures/ despite a comment saying scraper dumps must never be tracked. - CLAUDE.md quoted test counts and coverage percentages that had gone stale, which made a correct verification report look suspect. Gate thresholds kept. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DXPBfr1v7F71t6QcbHCaaT
This commit is contained in:
@@ -7,20 +7,49 @@ layer stays independent of any Python model objects.
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import logging
|
||||
from logging.config import fileConfig
|
||||
from typing import Any
|
||||
|
||||
from alembic import context
|
||||
|
||||
from migrations.url_resolution import coerce_sqlite_async, resolve_database_url
|
||||
|
||||
# Load the alembic.ini logging config without disabling existing loggers —
|
||||
# when running inside the FastAPI app the app's own loggers must not be reset.
|
||||
if context.config.config_file_name is not None:
|
||||
fileConfig(context.config.config_file_name, disable_existing_loggers=False)
|
||||
|
||||
logger = logging.getLogger("alembic.env")
|
||||
|
||||
|
||||
def _resolve_url() -> str:
|
||||
"""Resolve the database URL for this migration run.
|
||||
|
||||
This is the SINGLE place both the offline and online paths get their URL
|
||||
from — `alembic.ini`'s `sqlalchemy.url` is deliberately never read
|
||||
directly anywhere else, so Alembic can never silently target a different
|
||||
database than the one the running app actually uses.
|
||||
|
||||
The actual resolution order/logic lives in `migrations.url_resolution`
|
||||
(kept separate so it is unit-testable without an active Alembic
|
||||
context); this just wires Alembic's `-x` args and ini value into it.
|
||||
"""
|
||||
x_args = context.get_x_argument(as_dictionary=True)
|
||||
ini_url = context.config.get_main_option("sqlalchemy.url")
|
||||
url = resolve_database_url(x_args, ini_url)
|
||||
if not url:
|
||||
raise RuntimeError(
|
||||
"Could not resolve a database URL: no -x sqlalchemy.url, no "
|
||||
"APP_SQLITE_PATH-derived setting, and no alembic.ini sqlalchemy.url."
|
||||
)
|
||||
return url
|
||||
|
||||
|
||||
def run_migrations_offline() -> None:
|
||||
"""Emit SQL to stdout (used by `alembic upgrade --sql`)."""
|
||||
url = context.config.get_main_option("sqlalchemy.url")
|
||||
url = _resolve_url()
|
||||
logger.info("Resolved database URL (offline mode): %s", url)
|
||||
context.configure(
|
||||
url=url,
|
||||
target_metadata=None,
|
||||
@@ -41,10 +70,8 @@ async def run_migrations_online() -> None:
|
||||
"""Run migrations against a live DB connection."""
|
||||
from sqlalchemy.ext.asyncio import create_async_engine
|
||||
|
||||
url = context.config.get_main_option("sqlalchemy.url")
|
||||
# Coerce sqlite:// → sqlite+aiosqlite:// for async support.
|
||||
if url and url.startswith("sqlite:///"):
|
||||
url = url.replace("sqlite:///", "sqlite+aiosqlite:///", 1)
|
||||
url = coerce_sqlite_async(_resolve_url())
|
||||
logger.info("Resolved database URL (online mode): %s", url)
|
||||
|
||||
connectable = create_async_engine(url)
|
||||
async with connectable.connect() as connection:
|
||||
|
||||
@@ -0,0 +1,67 @@
|
||||
"""Pure database-URL resolution logic for Alembic migrations.
|
||||
|
||||
Kept separate from `env.py` so it is importable and unit-testable on its own:
|
||||
`env.py` has import-time side effects tied to an active Alembic
|
||||
`MigrationContext` (it calls `alembic.context.config...` at module scope) and
|
||||
cannot be imported outside of an actual `alembic` invocation. This module has
|
||||
no such dependency — it only takes plain values in and returns plain values
|
||||
out.
|
||||
|
||||
Resolution order implemented by `resolve_database_url` (first match wins):
|
||||
1. An explicit `-x sqlalchemy.url=...` argument — a documented way to point a
|
||||
one-off Alembic run at a scratch database without touching the environment.
|
||||
2. `APP_SQLITE_PATH` read directly from `os.environ` — but only when it is
|
||||
genuinely present and non-empty. This deliberately does NOT go through
|
||||
`app.config.Settings`: `Settings.sqlite_path` has a default
|
||||
(`"./foraldraledig.db"`), so a `Settings()` instance can never tell "the
|
||||
env var was set to the default's value" apart from "the env var was never
|
||||
set at all" — the default always wins, which made step 3 below dead code
|
||||
and meant a `sqlalchemy.url` set programmatically on the Alembic `Config`
|
||||
(e.g. `cfg.set_main_option(...)` in a test) was silently ignored in favour
|
||||
of whatever `./foraldraledig.db` resolved to, including the developer's
|
||||
real local database. Reading `os.environ` directly is the only way to
|
||||
distinguish "explicitly set" from "defaulted".
|
||||
Only the real `APP_SQLITE_PATH` environment variable counts here — a value
|
||||
from an `.env` file is deliberately NOT treated as "explicitly set" (this
|
||||
resolver never loads `.env`). `.env` is a developer convenience with
|
||||
quietly-applied defaults, exactly the kind of implicit source that caused
|
||||
this bug; requiring a real env var keeps "what database does this
|
||||
migration run touch" answerable by looking at the process environment
|
||||
alone, and it means an `.env` file can never silently retarget migrations
|
||||
for anyone running Alembic (locally or in CI) who does not also have that
|
||||
exact environment variable exported.
|
||||
3. The `sqlalchemy.url` value from `alembic.ini` — or from
|
||||
`Config.set_main_option("sqlalchemy.url", ...)`, which is indistinguishable
|
||||
from `alembic.ini` by the time it reaches `context.config.get_main_option`
|
||||
— as the fallback when neither of the above applies.
|
||||
|
||||
This is the single place that decides which database Alembic touches — never
|
||||
read `alembic.ini`'s `sqlalchemy.url` directly anywhere else, so migrations
|
||||
can never silently target a different database than the one the running app
|
||||
actually uses.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
|
||||
|
||||
def resolve_database_url(x_args: dict[str, str], ini_url: str | None) -> str | None:
|
||||
"""Resolve the database URL from -x args, a real APP_SQLITE_PATH env var,
|
||||
then the ini fallback."""
|
||||
explicit_url = x_args.get("sqlalchemy.url")
|
||||
if explicit_url:
|
||||
return explicit_url
|
||||
|
||||
sqlite_path = os.environ.get("APP_SQLITE_PATH")
|
||||
if sqlite_path:
|
||||
return f"sqlite:///{sqlite_path}"
|
||||
|
||||
return ini_url
|
||||
|
||||
|
||||
def coerce_sqlite_async(url: str) -> str:
|
||||
"""sqlite:/// → sqlite+aiosqlite:/// so the async engine can use it."""
|
||||
if url.startswith("sqlite:///"):
|
||||
return url.replace("sqlite:///", "sqlite+aiosqlite:///", 1)
|
||||
return url
|
||||
@@ -0,0 +1,44 @@
|
||||
"""Add parental_leave_fetch.transferable_days_reported.
|
||||
|
||||
Tracks provenance of `transferable_days_left`: True when it is FK's own
|
||||
reported figure (parsed from the "Dagar ni kan föra över" panel on the
|
||||
`min-foraldrapenning` e-service page, or a detailed manual entry that
|
||||
supplied it directly); False when `app.services.leave_snapshot.build_snapshot`
|
||||
derived it via the transferable-first fallback (manual entry with no split
|
||||
known, or a failed parse of that panel).
|
||||
|
||||
Existing rows default to false, which is accurate — every snapshot persisted
|
||||
before this migration was derived via the fallback, never FK-reported.
|
||||
|
||||
A plain `ADD COLUMN` suffices here (like 003's `take_pl_on_red_days`, unlike
|
||||
002's CHECK-constraint change which needed a full table rebuild) since
|
||||
SQLite supports adding a NOT NULL column with a constant DEFAULT directly.
|
||||
|
||||
Must stay byte-for-byte equivalent to the `parental_leave_fetch` table in
|
||||
`app/db/connection.py`'s `_FULL_SCHEMA` —
|
||||
`tests/integration/test_schema_parity.py` asserts exactly that.
|
||||
|
||||
Revision ID: 006
|
||||
Revises: 005
|
||||
Create Date: 2026-09-20
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from alembic import op
|
||||
|
||||
revision: str = "006"
|
||||
down_revision: str | None = "005"
|
||||
branch_labels: str | None = None
|
||||
depends_on: str | None = None
|
||||
|
||||
|
||||
def upgrade() -> None:
|
||||
op.execute(
|
||||
"ALTER TABLE parental_leave_fetch "
|
||||
"ADD COLUMN transferable_days_reported INTEGER NOT NULL DEFAULT 0"
|
||||
)
|
||||
|
||||
|
||||
def downgrade() -> None:
|
||||
op.execute("ALTER TABLE parental_leave_fetch DROP COLUMN transferable_days_reported")
|
||||
Reference in New Issue
Block a user