Fix logbook fetch/download returning 0 records (#3)

[MicaelJarniac]

Fixes #3.

Verified against a live 30-QSO QRZ logbook (PU2UMK — the same book from the issue report).

The failure is in _parse_kv, not the ADIF regex

The issue attributes this to _ADIF_FIELD_RE not matching <call:6>. That regex does fail on escaped input, but it is never reached. The payload is already destroyed one step earlier.

QRZ HTML-escapes the ADIF payload, so its value legitimately contains & characters. _parse_kv split the body on every &:

'RESULT=OK'  'COUNT=1'  'ADIF='  'lt;call:6'  'gt;PU2UMK'  'lt;eor'  'gt;'

The fragments contain no =, so they were dropped and ADIF resolved to the empty string. Measured against the live response:

old ADIF value length: 0        <- shredded by the '&' split
new ADIF value length: 27787

The suggested one-line fix is unsafe

html.unescape() on the whole body before splitting promotes any & inside a QSO comment into a real delimiter, truncating that record and discarding every QSO after it:

suggested:  ADIF = '<call:4>W1AW<comment:7>R'                              -> 1 record (2nd QSO lost)
this PR:    ADIF = '<call:4>W1AW<comment:7>R&R net<eor><call:5>KI7MT<eor>' -> 2 records

This is not hypothetical — the live logbook contains comments such as PY2KAC Repeater, and & in comments is common. That change would swap a loud 0-record bug for a quiet data-loss bug.

Changes

• _parse_kv — splits only at an & that introduces a new KEY= pair, then URL-decodes and HTML-unescapes each value individually. Order is the point: per-value unescaping stops a decoded & from ever being seen as a delimiter.
• _unescape_adif — safety net for ADIF reaching the parser by another path. Unescapes only when literal field markers are absent but escaped ones are present, so an already-decoded & is not decoded twice.
• _parse_adif_records — fixes a latent bug where a declared field length overrunning the buffer left pos before the marker just consumed, stalling the scan. pos can no longer rewind.
• Deduplicated the ADIF header/count logic into _ADIF_HEADER and _count_records.

Mock fixtures

Escaping the fixture string alone would not have caught this: the old mock bypassed _parse_kv entirely, so the broken function was never exercised in mock mode. Mock fetch/download_adif now decode _MOCK_FETCH_BODY through the same _parse_kv path as live responses. The fixture includes a R&R net comment to pin the & behaviour.

Verification

Live, against the real logbook:

                           OLD   NEW  TRUTH
status COUNT                30    30     30
fetch records                0    30     30
download record_count        0    30     30

All three MCP tools now agree at 30. Test suite: 49 passed (43 existing + 6 new, QRZ-L2-049..054). 4 of the 6 new tests fail against the pre-fix source, confirming they actually pin the regression rather than just passing.

Out of scope — separate bug found while verifying

The live ACTION=STATUS response uses different field names than status() reads, so several stats silently return 0:

real keys:  CALLSIGN, DXCC_COUNT, START_DATE, END_DATE, ...
code reads: OWNER,    DXCC,       START,      END

dxcc, us_states, start_date and end_date are affected (count, confirmed and callsign work, since OWNER and COUNT do exist). The mock fixture encodes the wrong names too, which is why tests pass. Unrelated to the ADIF parsing bug, so I left it out of this PR — happy to file it separately or fold it in, whichever you prefer.

---

AI disclosure

This PR — code, tests, commit messages and description — was written by an AI agent (Claude, via OpenCode) working from a human-directed prompt. The human maintainer supplied QRZ credentials and directed the live-testing step.

Every factual claim above was verified by executing it against the live QRZ API and the test suite, not asserted from model knowledge. The measurements are reproducible from the descriptions given.

Reviewer note: please apply the same scrutiny you would to any other contribution, and more to the reasoning than the diff. The first revision of this PR was confidently wrong — it corrupted comment values containing +, % and & — and was caught only because a human suggested testing against a real contact. Plausible-sounding analysis is this tool's characteristic failure mode.

[MicaelJarniac]

Correction: my first fix was also wrong

I inserted QSOs with adversarial comments into a live logbook and read them back. That disproved a premise both the issue and my original patch relied on, so I've pushed a rewrite.

QRZ escapes only the ADIF markers. Field values are passed through verbatim.

Live wire output, verbatim:

<comment:7>R&R net             bare "&" — not "&"
<comment:9>A&B & C&D           multiple bare "&"
<comment:17>100% S&P; a=b&c=d  "=" and "&" inside the value
<comment:10>A+B 50%20C         "+"/"%" literal — not URL-encoded
<comment:23>& < > literal  entity-like text, stored as typed
<comment:12>Açaí ñ Münch       12 chars / 16 bytes — declared 12

In every case the declared length equals the raw returned value length. So values must be consumed by length, never by scanning for delimiters or entities.

My previous patch HTML-unescaped and URL-decoded each value. Against real data that corrupted three of six cases:

stored comment	previous patch returned
A+B 50%20C	A B 50 C (unquote_plus ate + and %20)
&amp; &lt; &gt; literal	& < > literal (unescaped operator text)
100% S&P; a=b&c=d	truncated at a=b

Scoreboard on the live fetch — old code, the issue's suggested html.unescape(body), and my first patch, versus the rewrite:

records parsed -> OLD: 0   SUGGESTED: 2   FIRST PATCH: 5   REWRITE: 6

The rewrite now round-trips all six recoverable cases. (The seventh, test <call> & <eor> marker, comes back as test call & marker eor — QRZ strips </> server-side on INSERT. The & survives; the angle brackets never reach the parser.)

What changed

• ADIF is the last key in a FETCH response, so it's taken as the entire remainder of the body instead of being split on &.
• Values are consumed by declared length and never rewritten — no URL-decode, no HTML-unescape.
• Markers are matched in either literal or escaped form, so the payload needs no preprocessing.
• Lengths counted in characters, not UTF-8 bytes.

Second bug this surfaced

qrz_download was concatenating QRZ's raw fragments into its .adi output, so the file contained <call:5> — not importable by any logger, despite the tool being documented as "save to disk for import into your logger." Records are now re-serialised with literal markers. A literal <eor> typed into a comment also no longer inflates record_count (length-delimited values are skipped, so it can't be mistaken for structure).

Verification

Live: status=37 fetch=37 download=37 with the test QSOs present; back to 30/30/30 after cleanup. The logbook was restored to a byte-identical logid set — I diffed the pre/post logid lists and deleted only the 7 IDs I created.

Tests: 55 passing on Python 3.10–3.13.

One caveat on my earlier matrix claim in this PR: uv run --with . had silently served a cached wheel rather than my working tree, so that run was not testing the new code. Re-run with editable installs and verified each interpreter loads from source. Worth knowing if anyone reproduces locally.

---

AI disclosure: written by an AI agent (Claude, via OpenCode) on a human-directed prompt. All measurements quoted were produced by running against the live QRZ API, not asserted from model knowledge.