fix(0127): backfill status reported a completed archive as 0% done - #283
Merged
Merged
Conversation
GET /v1/backfill/status publishes progress_pct 0.0 and ledgers_remaining 63,795,748 beside status "completed" on production. Both fields assumed the stream walks forward from start_ledger, but the SDEX archive walks backward: sdex-backfill's progress.rs writes start_ledger = 1 and target_ledger = tip on every update and moves current_ledger DOWN via Current::SetBackward, so current_ledger is the oldest ledger reflected. Covered is [current, target]. progress_pct becomes (target - current) / (target - start), and ledgers_remaining becomes current - start — how far the floor still sits above genesis. Both are guarded on the seeded current_ledger = 0 placeholder, which the sink already treats as unset: without that check a brand-new row reads as 100% done, since a backward stream finishes at a LOW current_ledger. This matters beyond cosmetics. Tranche 2 acceptance criterion 5 sends a reviewer to this exact endpoint to read earliest_data_available. That number is correct; everything printed next to it said the backfill never started. Modified test: backfill_status_maps_both_streams asserted 60.96% and 22,342,964 remaining on a mid-run row. Those were the complement of the covered span, so they become 39.04% and 34,891,233. Intentional, not a regression. Verified against ClickHouse 26.3.10.60, the pinned production version.
karczuRF
added a commit
that referenced
this pull request
Sep 4, 2026
Found by code review on PR #283. progress.rs writes SetBackward(start) unconditionally at Phase::Completed, so a genesis-anchored chunk (--start 1 --end 20_000_000) sets current_ledger = 1 while status correctly stays running and the span above the chunk was never ingested. Pre-existing, but the corrected backward arithmetic in #283 turns it from a pessimistic misreading (0%) into an optimistic one (100%), on the endpoint Tranche 2 AC 5 sends a reviewer to. #283 contains it with a status guard; this task carries the actual fix and the three options for it.
Review of the first commit found the flip could be read optimistically where the old bug read pessimistically. Two real defects, both fixed. progress.rs writes SetBackward(start) unconditionally at Phase::Completed — the reached_genesis check below it gates only status. So the documented chunking pattern --start 1 --end 20_000_000 sets current_ledger = 1 while status correctly stays running, and the corrected arithmetic then published 100% with 31M ledgers missing. A non-completed stream is now held under PCT_RUNNING_CEILING, so it can never assert a completion its own status does not support. That contains the symptom; the column still cannot prove contiguity, which is task 0263. ledgers_remaining had no upper guard. current_ledger > target_ledger is reachable because sink.rs rewrites target_ledger on every write while mid-run updates keep current_ledger, so a chunked run without --tip collapsed the denominator below a stored floor and published more remaining than the span holds. Clamped, with the covered + remaining <= span invariant from 0176 now asserted across every reachable row shape. Also corrected the neighbouring OpenAPI text, which still described current_ledger as "Last ledger sequence ingested" and start_ledger as "First ledger sequence of this run" — forward readings that contradicted the new progress_pct description and would have reproduced the original bug; the sdex-backfill --tip warning, which claimed progress_pct "will over-report" when the corrected formula can err either way; and the worked example in both overviews, which showed 39.2 where the formula they document gives 39.04. Tests 5 -> 8. Verified against ClickHouse 26.3.10.60.
karczuRF
added a commit
that referenced
this pull request
Sep 4, 2026
PR #283's review found that docs/scf/milestone-1-evidence.md Figure 7 captures a /backfill/status response under the old forward formula and describes the archive as ~79% through the chain, where the corrected arithmetic reads 20.53%. The finding stands; the file does not change. M1 is submitted and accepted, and the document records what was sent rather than describing the system now. Editing it after the fact would change what the record says was claimed. Written down because a later session re-reading Figure 7 against a corrected endpoint will find the same discrepancy and reach for the same fix. Carries the consequence into 0128: an M2 exhibit of the same endpoint will look like a regression against Figure 7 and is not one.
karczuRF
added a commit
that referenced
this pull request
Sep 4, 2026
…answered AC 5 passes by six years. earliest_data_available reports 2015-11-18 against a 2022-01-01 bar, reconciled against the stored row, min(timestamp) on price_ohlcv_1d and the oldest active partition on all seven tiers rather than taken on trust — the writer's monotonic merge means an overstatement would have been permanent and invisible. Continuity is established by counting distinct days rather than candles: every calendar day from 2022-01-01 carries SDEX candles, leap years included, so there were no gaps to explain. AC 6 is answered with XLM and yBTC against Binance daily klines over 28 dates rather than the five asked for: median absolute deviation 0.06% and 0.48%, 27 of 28 within 5% on each. USDC is excluded with the reason stated in full — its series is entirely peg-derived, so it cannot fail the check and therefore cannot pass it. Three PRs merged: #283 the progress_pct direction fix, #284 the ADR 0009 language retirement, #285 the evidence report. Four follow-ups spawned, none cosmetic: 0263, 0264, 0265 and 0266. One item handed over as prose rather than a task, because it belongs to whoever owns the alarm: the Tranche 1 freshness alarm on last_push_at will fire forever now the stream is completed unless it is gated on status.
karczuRF
added a commit
that referenced
this pull request
Sep 4, 2026
0176 already owned the /backfill/status arithmetic bug and nobody read it first. It carries the same diagnosis PR #283 arrived at independently while closing 0127 — including the instruction not to repair the data, which was followed: the reader was changed and the stored current_ledger = 1 was left alone. Marked so the merged work is not redone. AC 1 and AC 4 close; AC 2, 3 and 5 do not. What remains is Defect 2 — a dead run that still advertises running, and completed_at predating last_push_at — which is a different problem from the arithmetic and is now the whole scope. Cross-linked from 0263, with the lesson recorded there: grep the backlog for the surface, not only for the symptom.
karczuRF
added a commit
that referenced
this pull request
Sep 4, 2026
#287) The RFP types Current Price as a float; we publish it — and every numeric field — as a decimal string, per the §3.3 precision design. The deviation was never written down, so it would have been improvised at review time. docs/scf/milestone-2-rfp-deviations.md now carries it, alongside the other two M2 deviations already decided: the X-Cache wording and USDC's exclusion from the backfill spot-check. Standalone rather than folded into 0128, which has not started — the same reasoning 0248 and 0122 settled. The argument does not rest on first principles. ADR 0011 records a measured production defect caused by exactly this: deriving through toFloat64 returned a close BELOW its own low by 1.343e-11 at BTC 1h, 0.92 of one float64 ulp, on a value carrying 19 significant digits against float64's 15-16. That was one internal conversion; publishing floats would impose it on every consumer, on every field. The affected assets are real — 7e-8 on RON, and a close of 5e-14 five ticks above the Decimal(38,14) floor — and 0120 already verified that the string form survives the JSON round-trip. Also states what the task's own wording did not: the answer covers every Decimal-valued field, not just the one the RFP names, and a consumer who wants a float can still parseFloat while the reverse is unrecoverable. The OpenAPI now says this on price_usd itself rather than only in the API-level blurb. Fixes one leftover from PR #283 in passing: the SdexStream schema description still said the archive walks the ledger history "in order".
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
GET /v1/backfill/statuspublishes this on production today:A completed stream reporting 0.0% progress and 63.8 M ledgers remaining.
Both fields assumed the stream walks forward from
start_ledger. The SDEX archive walks backward:sdex-backfill'sprogress.rswritesstart_ledger = 1andtarget_ledger = tipon every update and movescurrent_ledgerdown viaCurrent::SetBackward, socurrent_ledgeris the oldest ledger reflected. The covered span is[current, target], and the archive finishes atcurrent == start == 1— which the forward formula reads as zero.progress_pct→(target - current) / (target - start) * 100ledgers_remaining→current - start(how far the floor still sits above genesis)Both guard the seeded
current_ledger = 0placeholder, which the sink already treats as unset (resolve_current:Some(e) if e != 0). Without that check the direction flip inverts the bug — a brand-new row would read as 100% done, because a backward stream finishes at a lowcurrent_ledger. There is a test for exactly that.Why it matters beyond cosmetics
Tranche 2 acceptance criterion 5 sends a reviewer to this exact endpoint to read
earliest_data_available. That number is correct and passes — it reports2015-11-18, six years under the 2022-01-01 bar, and task 0127 reconciled it against real candle rows and the partition census. Everything printed beside it said the backfill never started.Tests
Five new unit tests on the two helpers: the completed production row, a mid-run row, the seeded-
0sentinel, a zero span, and an out-of-rangecurrentabove the tip.Modified test — intentional, not a regression.
backfill_status_maps_both_streamsasserted60.96%and22,342,964remaining for a mid-run row (start 1,current 34,891,234,target 57,234,198). Those were the complement of the covered span, so they become39.04%and34,891,233.Verified against ClickHouse 26.3.10.60, the pinned production version — all 10
endpoints_itintegration tests pass, plus the fullprices-apisuite.Also updated
The OpenAPI field descriptions and the route summary, which both described the archive as walking "in order". A reader following the published formula would have reproduced the bug.
Not in scope
Task 0127 found a second, larger problem on the same criterion — USDC's
/ohlcvseries is entirely peg-derived (trade_count: 0on all 2,042 points, exactly1on 1,865 of them, including 2023-03-11 when USDC really traded ~$0.87). That is a decision, not a code fix, and stays in the task.