uci: DATA_ACC is an accept, not a per-byte advance (#144 item 1)

[JC-000]

Closes item 1 of #144.

The change

uci_drain_resp pulsed $02 → $DF1C after every byte it read. That bit is DATA_ACC, the end-of-transfer accept — not a FIFO advance. Register API v1.1 §2.4.1: writing it "causes the transfer of the data/status queues to be aborted and reset". And the response queue needs no host action at all — command_protocol.vhd advances response_pointer on the C64 read strobe:

when c_cif_slot_response =>  response_pointer <= response_pointer + 1;

So the loop read exactly one byte, saw DATA_AV drop, and returned "drain complete" — and it took the status line with it, before the caller's next step (uci_drain_status, #147) could read the firmware's own reason. Every call site is drain_resp → drain_status → uci_ack, so this hit net_poll's @pe error path and the ring-full early exits: precisely the moments the explanation is worth having.

Three instructions deleted. The lda UCI_RESP_DATA keeps its fence, so the loop still leaves one uci_fence between consecutive UCI register accesses — the same spacing every other access in this adapter uses, and the same shape uci_drain_status has carried since #148. (Worth knowing: the loop used to have two fences between the response read and the next status read, so the inter-access gap in this one loop halves. It is still at the documented contract, and the loop is now identical in shape to the hardware-verified status drain.)

The 5 s CIA-TOD bound stays. network_target.h documents that a reply of exactly CMD_MAX_REPLY_LEN leaves DATA_AV asserted indefinitely; our 512 B cap cannot reach it, but the bound is the backstop.

Item 2 is already satisfied here. #144 warns that removing the per-byte pulse can silently remove a path's only accept. It does not in this tree: all 12 drain sites in net.s — including the SOCKET_WRITE path — already run drain_resp → drain_status → uci_ack, so the accept is where it belongs and there is exactly one per transaction.

Zero-byte truth fixes — the trap #144 predicted

• UCI_CTRL_NEXT_DATA → UCI_CTRL_DATA_ACC. Our comment was ; advance response data FIFO — the exact wording UCI handshake: DATA_ACC is an accept, not a per-byte advance — three defects it causes #144 flagged, wrong on both counts, and the thing that propagated the misunderstanding. Every use updated.
• uci_cmd.s's file header and uci_read_resp_bytes's header both described the drains as "ACKing each" byte. Neither routine does.

uci_errors.inc's $82/$84/$85/$86 comments are untouched — a doc lane owns those.

Build

BACKEND=uci USE_NISTCURVES_ONCHIP=1, make clean between:

	PRG sha256	UCI_CODE	NET_CODE free
before	1bb5fad91273b5e93f26611a80bb406d76fdafa3dae68185dc0884eb5c5d5fa7	$081B	59 B
after	f7572c94252172b7c7388e06767bff0e04bee6347e757be0f185dd90ab974a3c	$0813	67 B

−8 bytes, exactly as predicted. PRG size is unchanged at 62,977 B (the overlay pad absorbs it); the hash is the evidence. ip65 was deliberately not built here — this is a nested worktree and .incbin resolves against the CWD. The three touched .s/.inc files are UCI-only and are not assembled under ip65; pytest.ini and the new test are backend-neutral.

The test, and what it does not prove

tools/test_uci_data_acc.py. There is no VICE path for UCI — $DF1B-$DF1F is unmapped in the emulator — and no hardware in CI, so a conventional behavioural test is not available. Rather than write a shape assertion, this executes the shipped machine code: it lifts uci_drain_resp, uci_drain_status and uci_ack out of build/c64-https.prg at their build/labels.txt addresses and runs them on a small 6502 interpreter (in-file, ~300 lines, NMOS subset) against a model of the Command Interface built from the two sources #144 cites — reads auto-advance each queue; $DF1C reports DATA_AV/STAT_AV while its queue has bytes; writing $02 ends the data phase and empties both.

Three checks:

1. the regression — after uci_drain_resp, no DATA_ACC was written, the whole response was consumed, the status queue is untouched, and uci_drain_status then captures the firmware's full line into uci_status_buf / uci_status_seen.
2. a control — with nothing queued for drain_resp, the capture works, so a failure in (1) cannot be blamed on drain_status or on the harness.
3. an anti-regression pin for UCI handshake: DATA_ACC is an accept, not a per-byte advance — three defects it causes #144 item 2 — uci_ack still writes exactly one DATA_ACC and still ends the transfer.

Red/green, honestly: check 1 was recorded failing against the pre-fix PRG (1bb5fad9…), on the "wrote DATA_ACC 1 time(s)" assertion, and passes after. Checks 2 and 3 passed both before and after — 2 is a deliberate control and 3 is a pin on something this change could have broken; neither is offered as evidence of the fix.

What it does not prove. The FPGA model is a reading of the Register API text and the VHDL, not a measurement — if the model is wrong, the test is wrong with it. It says nothing about register timing (the uci_fence floor, and specifically the halved inter-access gap noted above), nothing about firmware behaviour above the register layer, and nothing about the other two defects in #144. Runs in 0.12 s; registered in pytest.ini testpaths, so bare pytest at root goes 31 → 34 passed.

The hardware run that would close it

On a U64E or C64U, any HTTPS rig that reaches an error path — tools/uci/rig_https_live.py against github.com is enough, since net_poll's idle polls answer $FFFF and take the @hdr_done drains on every cycle:

• Confirms the fix: the run completes as before (HTTP 200), and a post-mortem uci_status_len / uci_status_buf read shows a status line longer than one byte — the firmware's own text, e.g. 02,NO DATA: 11. Note uci_drain_status filters 00, and 02, lines out of the sticky slot, so read uci_status_seen (non-sticky, counts every byte seen) for the length oracle; it should be the full line length, not 1.
• Refutes it: a handshake that used to pass now fails, or NO_SOCKET/$89 WAIT_TIMEOUT appears where it did not before. That would mean the halved inter-access gap in the drain loop is below the FPGA floor at that clock, and the fix needs a fence restored rather than the pulse.
• A boot_check.py + rig_http_local.py pass first is the cheap ladder rung; either regressing points at the timing, not at the queue semantics.

🤖 Generated with Claude Code

[JC-000]

Verified independently

Built from the branch head in the main checkout: PRG sha256 f7572c94252172b7c7388e06767bff0e04bee6347e757be0f185dd90ab974a3c, matching your report. Baseline 1bb5fad9…. PRG size unchanged at 62,977 B (OVERLAY_FILE_PAD absorbs it), so the hash is indeed the only evidence — noting that because size-based checks would read this as a no-op.

On the fence caveat — the framing is better than "halves the gap"

You flagged this responsibly and it deserves a precise answer rather than a hedge. Reading the post-fix loop:

@drn_have:
        lda UCI_RESP_DATA
        jsr uci_settle
        lda CIA_TOD_HOUR       ; TOD bookkeeping — negligible vs a fence
        lda CIA_TOD_TENTHS
        ...
        jmp @drn_loop          ; -> lda UCI_STATUS

The gap between the $DF1x accesses is now one fence plus a few cycles of TOD bookkeeping, where it used to be two fences plus an intervening write. So yes, roughly halved in wall-clock.

But the useful statement is what the remaining gap is, not what it was: one fence is the designed operating point. Per CLAUDE.md, OUTER=5 INNER=217 is ~85 µs, against a floor bracketed at 51.6 µs FAIL / 62.9 µs PASS on the C64U — "INNER=217 chosen for ~35% margin". Every other $DF1C-$DF1F access site in the adapter runs on exactly one fence.

So this loop was carrying accidental extra margin that no other site has, purely as a side effect of the defect. Removing the pulse brings it into line with the contract rather than dropping it below one. If 85 µs turns out to be inadequate here, it is already inadequate everywhere — which is precisely #147's open hypothesis (the constant was bracketed against core 1.4E; the U64E now runs 1.4F).

That reframes the risk: this PR does not introduce a new timing exposure, it removes an accidental cushion on one loop. Worth confirming on hardware regardless, and the run you named is the right one.

Sequencing

Two independent arguments now converge on the same order, so I am holding this for the hardware session rather than merging first:

1. Yours — the drain loop wants a real run on an error path.
2. rig_https_live against github.com fails ~1 run in 3 on the U64E, on master as well as on branches #147's — its Phase 1 baseline should be taken on current master, before this lands, so the measurement stays comparable to the 08-29 fence A/B. Landing this first would change the instrument between baseline and measurement.

Plan: #147 Phase 1 baseline on master → merge this → re-run the identical arm. That is one extra ~7-minute arm and it turns "#144 item 1 is obviously right" into the first measured claim in either issue.

The self-instrumenting test is a genuinely good piece of work — lifting the shipped machine code out of the PRG at its labels.txt addresses and running it on an interpreter against a register model is a real red/green oracle for a path that has no VICE route at all. Your statement of what it does not prove (the model is a reading of the VHDL, not a measurement) is exactly the right caveat to attach.