Styling fix and scraper bug fix
This commit is contained in:
@@ -0,0 +1,167 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user