Plan: Implement shared download primitives (10.1.2) - #199
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Warning Your free Security trial is over. An organization admin can activate billing to continue. Comment |
Reviewer's GuideAdds a new ExecPlan document for roadmap item 10.1.2 describing shared download primitives for benchmark datasets and wires it into the documentation contents index; no code changes, only documentation for a multi-stage implementation plan. Sequence diagram for resumable verified download via RangeFetcher and PartialStoresequenceDiagram
actor DatasetRecipe
participant RecipeContext
participant TransferPorts
participant DownloadDomain
participant RangeFetcher
participant PartialStore
participant PartialSlot
DatasetRecipe->>RecipeContext: transfer()
RecipeContext-->>DatasetRecipe: TransferPorts
DatasetRecipe->>DownloadDomain: download_verified(TransferPorts, DownloadRequest)
DownloadDomain->>PartialStore: acquire(CacheKey)
PartialStore-->>DownloadDomain: PartialSlot
DownloadDomain->>PartialSlot: staged_len()
DownloadDomain->>PartialSlot: expectation()
DownloadDomain->>DownloadDomain: decide_resume(ResumeContext)
alt ResumeFrom
DownloadDomain->>RangeFetcher: open_range(RangeRequest)
RangeFetcher-->>DownloadDomain: RangeResponse
DownloadDomain->>PartialSlot: append(&[u8])
else DiscardAndRestart
DownloadDomain->>PartialSlot: reset(0, StagedExpectation)
DownloadDomain->>RangeFetcher: open_range(RangeRequest)
RangeFetcher-->>DownloadDomain: RangeResponse
DownloadDomain->>PartialSlot: append(&[u8])
end
DownloadDomain->>PartialSlot: commit(ObjectKey)
DownloadDomain-->>DatasetRecipe: DownloadOutcome
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Draft the execution plan for roadmap item 10.1.2: pinned source URLs, checksum verification, resumable transfers, and archive extraction in `chutoro-bench-datasets`. The plan was produced with a six-lens design review and validated against the working tree and live upstreams rather than assumption. Eight empirical checks shaped it: - Every upstream the project actually uses honours HTTP range requests with strong validators and no content coding, so resume has a working primary path rather than only a fallback. - `sha2 0.10` cannot persist hasher midstate, so a flat digest would force a full prefix re-read on every cross-process resume. Digests are chunked. - `cap-std` exposes no file locking, and `into_std()` returns the `std::fs::File` that Whitaker bans in this crate, so locking goes through `rustix::fs::flock` on the borrowed descriptor. - `Checksum` is currently uninhabited, so `SourceSpec.checksum` is a field that can provably never hold a value. - `RecipeError` is exactly 32 bytes with its payload budget already spent, so every rich failure payload must be boxed. - No roadmap dataset ships `.bz2` or `.xz`, while canonical GloVe ships `.zip`, which is outside the item's declared format list. - The repository has no advisory or licence gate, while this item roughly doubles the crate's dependency count. - `make verus` is a blocking pull-request gate whereas Kani is nightly only. Notable design decisions recorded in the plan: HTTP protocol types stay out of the domain layer so roadmap 10.1.4's object-store adapter need not fabricate status codes; `RecipeContext` gains a builder accessor so the streaming ports are reachable from a recipe at all; `Option<Checksum>` becomes a mandatory `Integrity` field with a greppable unpinned escape hatch; the signature adapter defers to 10.1.6 since no dataset in the backlog ships a signature; and the two originally proposed Verus proofs are replaced by one proof on the jitter arithmetic plus a proptest state machine over attempt sequences. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
c2911b1 to
b77564e
Compare
deny.toml and a cargo-deny CI job before the dependency surface grows
#214
The first draft said "Kani is nightly only". Issue #202 establishes the sharper position: `make kani` is invoked by no workflow at all, and `make kani-full` runs only post-merge on `main` and currently does not complete owing to CBMC budget exhaustion. The verification story is unchanged — two Kani harnesses and one Verus proof — but the plan now states plainly that the harnesses are developer tooling rather than a merge gate, must be run by hand at each stage boundary, and that a green pull request is not evidence they passed. Also records that the plan's findings were filed as issues #208 to #217. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Draft ExecPlan for roadmap item 10.1.2 — pinned source URLs, checksum verification, resumable transfers, and archive extraction (
.gz,.bz2,.xz,.tar) inchutoro-bench-datasets.docs/execplans/10-1-2-shared-download-primitives.mdThis is a plan only. No implementation has begun, and none will until the plan is approved.
How the plan was produced
A six-lens design review stress-tested the design before any code was written, and every load-bearing claim was checked against the working tree or live upstreams rather than assumed. Eight checks changed the design:
Range?206, correctContent-Range,Accept-Ranges: bytes, strong ETags, noContent-Encodingsha2 0.10persist hasher state?SerializableStatearrived indigest 0.11cap-stdoffer file locking?into_std(), which returns thestd::fs::FileWhitaker bans hererustix::fs::flockon the borrowed descriptorChecksuminhabited?#[cfg(any())]SourceSpec.checksumprovably can never hold a value; migration is in-crate onlyRecipeError?.bz2/.xz?.zip, which is out of scope.zipraised as a note against 10.3.10deny.toml, no advisory check in any of eight workflowsmake verusblocking?Design decisions worth reviewer attention
RangeOutcome; the domain parses header values but never branches on a status code. Without this, roadmap 10.1.4'sobject_storeadapter would have to fabricate HTTP status codes for the domain to inspect.RecipeContextgains a builder accessor. As originally designed the streaming ports would have shipped complete, verified, and unreachable fromDatasetRecipe::fetch.RecipeContext::newkeeps its exact three-argument signature, so all eight existing call sites are untouched.Option<Checksum>becomes a mandatoryIntegrityfield with a greppableUnpinned { justification }escape hatch, andsize_byteslives in the same variant aschecksumso the length-before-digest retry rule is true by construction.Integrity::Signedstill ship, so verification stays expressible.(sample * span) >> 64 <= span: nonlinear, unbounded in both operands, unsamplable by proptest and infeasible for Kani. A proptest state machine over attempt sequences covers the resume property against the real code.Validation
make markdownlint(0 errors, including the spelling gate) andmake nixieboth pass. No source code is touched.References
docs/roadmap.md§10.1docs/benchmark-dataset-retrieval.md§3.2docs/execplans/10-1-1-chutoro-bench-datasets-and-dataset-recipe-trait.mddocs/adr-004-bench-dataset-recipe-trait.md🤖 Generated with Claude Code
Summary by Sourcery
Document the proposed implementation plan for trusted, resumable benchmark dataset downloads and bounded archive extraction without adding implementation code.
Enhancements:
Documentation: