168 lines
7.1 KiB
Markdown
168 lines
7.1 KiB
Markdown
# FK Scraper — "zero days" bug fix plan
|
||
|
||
**Status:** Ready for implementation · **Difficulty:** Junior-friendly (one senior-flagged optional item)
|
||
**File to edit:** `backend/app/scrapers/fk_scraper.py`
|
||
**Tests:** `backend/tests/unit/test_fk_scraper.py`
|
||
|
||
---
|
||
|
||
## TL;DR
|
||
|
||
The scraper navigates correctly, clicks *Ansök för …*, and reaches the `#/valjdagar`
|
||
page. The bug is **a race condition on that final page**: the
|
||
`[data-test="stepDagarKvar"]` panel renders its skeleton with **placeholder zeros
|
||
first**, then fills in the real numbers after an async data fetch. The scraper reads
|
||
the moment the container exists — so it grabs the zeros and reports `days_left=0`,
|
||
`total_days=0`, etc., and exits "successfully".
|
||
|
||
So your hypothesis was right in spirit (the data isn't fully loaded when we read) but
|
||
slightly off on the *where*: the click and navigation already work — it's the
|
||
post-navigation **hydration** of the data panel that we're not waiting for.
|
||
|
||
---
|
||
|
||
## Evidence (so you don't have to re-derive it)
|
||
|
||
1. **The selectors are correct.** The fully-loaded capture
|
||
`captures/05_foraldrapenning_overview.html` contains the real values using the exact
|
||
selectors the scraper already uses:
|
||
- `visa-dagar__dagar-kvar` → `164`, `236`, `39`
|
||
- `av 240 dagar`, `av 480 dagar`
|
||
- tier `(119 dagar)`, `(45 dagar)`, `(146 dagar)`, `(90 dagar)`
|
||
So this is **not** a selector problem.
|
||
|
||
2. **The logs show a silent all-zero read, not an error.** From your run:
|
||
```
|
||
days_left=0
|
||
total_days=0 days_used=0
|
||
niva_texts raw=['(0 dagar)', '(0 dagar)']
|
||
dubbeldagar_left=0
|
||
```
|
||
Every selector matched and returned text — the text was just the placeholder `0`.
|
||
|
||
3. **Nothing catches it.** In `_extract_parental_leave`, the consistency checks all
|
||
pass for an all-zero panel: `days_used = 0 - 0 = 0`, and the tier cross-check
|
||
`0 + 0 == 0` holds. So garbage is happily persisted.
|
||
|
||
**Where it goes wrong in code:** `_do_extract` (around lines 391–394) waits for
|
||
`_SEL.STEP_DAGAR_KVAR` to *exist*, then immediately calls `_extract_parental_leave`.
|
||
`wait_for_selector` resolves on DOM presence, not on data being populated.
|
||
|
||
---
|
||
|
||
## The fix
|
||
|
||
Two changes, both small. Item 1 is the actual fix; item 2 is a cheap safety net so we
|
||
never silently persist zeros again.
|
||
|
||
### 1. Wait for the data to hydrate (primary fix) — `_do_extract`
|
||
|
||
After the panel container appears, wait until the **total days** value is positive
|
||
before extracting. Total is always `240` (for you) / `480` (for the child) and is
|
||
**never legitimately 0**, which makes it a safe, reliable "data has loaded" signal.
|
||
(Don't key off `days_left` — that one *can* legitimately be 0 for a parent who has used
|
||
everything.)
|
||
|
||
In `_do_extract`, after the existing `wait_for_selector(_SEL.STEP_DAGAR_KVAR)`:
|
||
|
||
```python
|
||
# Wait for the valjdagar data page container to mount
|
||
await page.wait_for_selector(_SEL.STEP_DAGAR_KVAR, timeout=30_000)
|
||
|
||
# The panel mounts with placeholder zeros, then hydrates from an async fetch.
|
||
# Wait until the "av N dagar" total is positive — total is always 240/480, never 0,
|
||
# so this is a reliable "real data has arrived" signal (days_left can legitimately be 0).
|
||
logger.debug("[%s] waiting for valjdagar data to hydrate", state.session_id)
|
||
await page.wait_for_function(
|
||
"""(sel) => {
|
||
const el = document.querySelector(sel);
|
||
if (!el) return false;
|
||
const m = el.textContent.match(/av\\s+(\\d+)\\s+dagar/);
|
||
return m && parseInt(m[1], 10) > 0;
|
||
}""",
|
||
arg=_SEL.FOR_DIG_DISCRETE,
|
||
timeout=30_000,
|
||
)
|
||
logger.debug("[%s] valjdagar data hydrated (url=%s)", state.session_id, page.url)
|
||
```
|
||
|
||
Notes for whoever implements this:
|
||
- `page.wait_for_function` runs the JS in the browser and polls until it returns
|
||
truthy or times out. The `arg=` value is passed in as the function's parameter.
|
||
- `_SEL.FOR_DIG_DISCRETE` is `"[data-test='forDig'] .visa-dagar__discrete"`, whose text
|
||
is `"av 240 dagar kvar efter ansökan"`. Same string `_extract_parental_leave` already
|
||
parses, so we're waiting on exactly the value we're about to read.
|
||
- Keep the existing `wait_for_selector` line — mount first, then hydrate.
|
||
|
||
### 2. Guard against placeholder/zero data (safety net) — `_extract_parental_leave`
|
||
|
||
Even with the wait, add a loud failure if we somehow still read the placeholder, so we
|
||
fail visibly instead of persisting zeros. Put this right after `total_days` is parsed
|
||
(around line 430):
|
||
|
||
```python
|
||
if total_days == 0:
|
||
raise ValueError(
|
||
"FK panel returned total_days=0 — data had not hydrated when read "
|
||
"(placeholder state). The hydration wait in _do_extract should prevent this."
|
||
)
|
||
```
|
||
|
||
This complements the existing `days_left < 0` and reserved-days checks.
|
||
|
||
### 3. (Optional, senior) Save debug artifacts on this failure path
|
||
|
||
`_save_debug_artifacts` already exists and is called from `_run_session`'s `except`
|
||
block — so once item 2 raises, you'll automatically get HTML + screenshot + console
|
||
dumped to `settings.scraper_debug_dir` (if configured). Just confirm
|
||
`APP_SCRAPER_DEBUG_DIR` is set when reproducing, and you'll capture the exact DOM state
|
||
if it ever recurs.
|
||
|
||
---
|
||
|
||
## Optional follow-up (separate ticket — senior, NOT part of this fix)
|
||
|
||
`_do_extract` clicks `_SEL.ANSOK_BUTTON` via `.first`. If a user has **multiple
|
||
children**, there are multiple *"Ansök för <name>"* buttons and `.first` picks the wrong
|
||
one. We already carry `state.child_id`; a future change should resolve the correct
|
||
button by child name rather than blindly taking the first. Out of scope here — flag it,
|
||
don't fix it in this PR.
|
||
|
||
---
|
||
|
||
## Tests to add (`backend/tests/unit/test_fk_scraper.py`)
|
||
|
||
The existing tests use a regex-backed mock page (`_make_mock_page`) and never touch a
|
||
real browser — follow that pattern. Add:
|
||
|
||
1. **Zero-guard test:** build a mock page whose `FOR_DIG_DISCRETE` returns
|
||
`"av 0 dagar kvar efter ansökan"` and assert `_extract_parental_leave` raises
|
||
`ValueError` (mirror `test_total_days_parse_failure_raises` at line ~287).
|
||
|
||
2. **Hydration-wait test (`_do_extract`):** mock `page.wait_for_selector` and
|
||
`page.wait_for_function` as `AsyncMock`s and assert `_do_extract` awaits
|
||
`wait_for_function` **before** extracting. A simple version: give the mock page a
|
||
`wait_for_function` that flips a fixture's discrete text from `"av 0 dagar"` to
|
||
`"av 240 dagar"` on first call, and assert the final result has `total_days == 240`.
|
||
(If wiring the ordering is fiddly, asserting `wait_for_function` was awaited once with
|
||
`arg=_SEL.FOR_DIG_DISCRETE` is enough to lock in the behavior.)
|
||
|
||
Keep the all-zero capture out of the happy-path fixture — `05_foraldrapenning_overview.html`
|
||
is the fully-loaded page and should keep passing the existing extractor tests unchanged.
|
||
|
||
---
|
||
|
||
## How to verify
|
||
|
||
Backend checks (from `backend/`):
|
||
```bash
|
||
uv run ruff check .
|
||
uv run ruff format --check .
|
||
uv run pytest
|
||
```
|
||
Then a live, non-headless run (from `frontend/`): `npm run login`. Watch for the new
|
||
`valjdagar data hydrated` debug line, and confirm the logged figures match what's on
|
||
screen (e.g. `days_left=164`, `total_days=240`, dubbeldagar `39`).
|
||
|
||
After editing, run `graphify update .` to keep the graph current.
|