Wire DataPipe egress with per-round incremental saves (C&WG) - #16
Open
htsukamoto5 wants to merge 2 commits into
Open
Wire DataPipe egress with per-round incremental saves (C&WG)#16htsukamoto5 wants to merge 2 commits into
htsukamoto5 wants to merge 2 commits into
Conversation
htsukamoto5
marked this pull request as draft
July 31, 2026 15:25
Member
Author
htsukamoto5
force-pushed
the
feat/datapipe-egress-and-identifiers
branch
from
July 31, 2026 15:54
7879926 to
6fbae83
Compare
htsukamoto5
changed the base branch from
main
to
feat/prolific-identifiers-and-codes
July 31, 2026 15:54
htsukamoto5
marked this pull request as ready for review
July 31, 2026 15:55
Closes #3. Adds the save target this file has never had. Until now the only egress was a download button the participant had to choose to click, so every dropout, lobby timeout, and spectator contributed nothing — precisely the sessions needed to characterise attrition. Builds on the CONFIG block and identifiers from the parent branch rather than duplicating them; the save-policy values join that block with their DECISIONS.md rows (A3, A4, B4). The retry classifier is the part worth reviewing. It was written against DataPipe's server source (jspsych/datapipe, functions/src/api-data.ts) rather than the plugin's example snippet, because the semantics decide whether retrying is correct or actively harmful: 201 uploaded. 202 means the OSF upload FAILED but DataPipe persisted the data and queued its own server-side retry — a success for us, and retrying it would duplicate rows. 400 OSF_FILE_EXISTS is the one path that is not queued and where data is genuinely dropped, hence a nonce in every filename. Other 4xx (session limit, validation, unknown experiment) are configuration errors, not transient faults; retrying burns the redirect budget and still loses the data. Only 429/5xx/network are retried, with backoff honouring Retry-After. Classification keys on status class rather than error strings, so upstream adding a new 4xx — the in-flight provider-migration branch adds PROVIDER_NOT_CONNECTED — is handled correctly with no change here. Chunks are disjoint and carry row_range, so reassembly is a concatenation and a duplicate chunk is detectable rather than silently merged. Failed chunks return their rows to the queue for a later flush: duplicates are recoverable in analysis, missing rows are not. The nonce is fixed for the lifetime of a save, so retries within one save reuse the filename and are idempotent. If attempt 1 lands at OSF and only its response is lost, attempt 2 gets OSF_FILE_EXISTS; that now counts as saved, because treating it as failure would requeue the rows and re-send them under a fresh nonce, duplicating them. A collision on the first attempt is a genuine clash with another session and stays terminal. flush() races the save against a redirect budget so no exit can strand a participant waiting on OSF, and pagehide uses keepalive to catch tab-close and connection loss. Wired to the exits that exist: per-round, completion, and spectator. The abort (#5) and no-match (#6) exits are one flush call each once those screens exist. Adds tests/pipeline.test.mjs: 34 checks, no dependencies, no browser. Covers chunking, every response class, requeue-on-failure, concurrent saves, filename uniqueness and fallbacks, the redirect budget, idempotent replay, inert behaviour when unconfigured, and end-to-end reassembly of a partial lossy session. For analysis: chunk_seq may contain gaps, since a failed save consumes a sequence number and its rows reappear later. A gap is not missing data — row_range is the gap-free contiguity check. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
htsukamoto5
force-pushed
the
feat/datapipe-egress-and-identifiers
branch
from
July 31, 2026 16:27
6fbae83 to
f8778d1
Compare
Blocking bug from review. `savedThrough -= rows.length` subtracts a count from a shared cursor, which is only correct if the failing save is the most recent claimant. Saves do not fail in the order they were issued. Reproduced: save A claims rows 0-2 and fails slowly, save B claims 3-4 and succeeds, a third save follows. row ranges [[3,4],[2,5]] overlapping rows landed [3,4,2,3,4,5] MISSING [0,1] never re-sent DUPLICATED [3,4] Rows 0 and 1 are gone permanently. Worse, the failure is silent: the overlapping ranges are exactly what breaks the contiguity invariant that analysis was told to check, so the detection mechanism stops holding at the moment it is needed. And it is the ordinary case, not an exotic one — the per-round save is not awaited and can retry for seconds, so a slow round-6 save overlapping the end-of-run flush is how most sessions end. savedThrough is now a high-water mark that only advances. Failed saves push their [start, end] ranges onto `pending`, and each save claims everything pending plus everything new. Chunks therefore emit `row_ranges` — a LIST of inclusive ranges — because a chunk may carry rows reclaimed from an earlier failure that are not contiguous with its new ones. Also from review: - The Prolific PID is out of the filename and lives in the payload only. OSF file listings are browsable without opening any file, so a PID in a filename publishes a directory of participant identifiers — a broader exposure than the same value inside a row, with D3/D6 still open with IRB. Reconciliation is unaffected; it reads the payload. - classify() accepts any 2xx rather than 201/202 exactly. Treating an unexpected 200 as failure would requeue rows that did land and duplicate them — the exact failure this module works hardest to avoid, triggered by nothing worse than an upstream tightening. - The completion screen no longer promises a retry that may not happen. A 202 is retried by DataPipe server-side; a flush that merely ran out of redirect budget may be a dead network, where nothing retries. - configured() uses a truthiness check, so an undefined key is falsy rather than a TypeError. - The test's script extraction fails with a readable message instead of a destructuring throw if the file gains a second <script> block. - dyadKey() is documented as defensive-only now that DYAD_ID has its own fallback, so it is not preserved as load-bearing. Tests: 39 checks, up from 34. Adds the out-of-order failure regression (the case the previous concurrency test missed, because both its saves succeeded), and asserts the PID is absent from filenames but present in payloads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
htsukamoto5
force-pushed
the
feat/datapipe-egress-and-identifiers
branch
from
July 31, 2026 17:51
cd9a3d3 to
5ad35d0
Compare
This was referenced Jul 31, 2026
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
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.
Closes #3.
Adds the save target this file has never had. Until now the only egress was a download button the participant had to choose to click, on a screen that also said "You may close this tab" — so every dropout, lobby timeout, and spectator contributed nothing. Those are precisely the sessions needed to characterise attrition.
Stacked on #14 (
feat/prolific-identifiers-and-codes), notmain. It builds on theCONFIGblock and identifiers that PR adds rather than duplicating them — the save-policy values join that block, each annotated with itsDECISIONS.mdrow. Review #14 first; this diff is only the egress layer and its tests.C&WG only. Hawkins is untouched on purpose (A2).
The decisions this encodes are suggestions, not settled calls
SAVE_PER_ROUND)FLUSH_ON_ABORT)limitSessionsis off. DataPipe's counter increments per save call, so per-round saving spends ~7 sessions per participant; a limit sized to participants would cap the run about a seventh of the way through and drop every later save silently.Changing any of these is a one-line edit in
CONFIG; nothing below hardcodes an equivalent literal.The retry classifier is the part worth reviewing
Written against DataPipe's server source (
jspsych/datapipe,functions/src/api-data.ts) rather than the plugin's example snippet, because the response semantics decide whether retrying is correct or actively harmful:201202400 OSF_FILE_EXISTS400othersPROVIDER_NOT_CONNECTED— configuration errors, not transient faults429,5xx, networkRetry-AfterClassification keys on status class, not error strings. That is what makes it robust to the in-flight
provider-migrationbranch upstream, which moves duplicate detection into a Firestore collision cache and makes OSF one provider among several — the client contract is unchanged, and any new 4xx is terminal by construction.The subtle one
The nonce is fixed for the lifetime of a save, so all retries within one save post the same filename — that is what makes them idempotent. But if attempt 1 lands at OSF and only its response is lost to a network fault, attempt 2 gets
OSF_FILE_EXISTS. Classifying that as terminal requeues the rows and re-sends them under a fresh nonce, duplicating them in the dataset — exactly what the nonce exists to prevent.A collision from attempt 2 onward now counts as saved. A collision on the first attempt is a genuine clash with another session and stays terminal. This was found by writing the reassembly test, not by reading the code.
Other behaviour
row_range, so reassembly is a concatenation and a duplicate chunk is detectable rather than silently merged.flush()races the save against a redirect budget, so no exit path can strand a participant waiting on OSF before they can submit (Prolific plumbing: completion codes and submission redirect #7). The submit button is wired before the flush resolves.pagehide+keepalivecatches tab-close, back-navigation, and connection loss (A3).keepalivecaps bodies at 64KB, which per-round chunks stay well under — another reason A4's chunking is load-bearing rather than merely tidy.PROLIFIC_PID, fall back to the adapter participant id locally, anddyad_idfalls back toSEEDso a filename never contains the literalnull.Wired exits
Per-round, completion, spectator, and
pagehide. The two that don't exist yet are onePipeline.flush(label)call each once their screens land: the partner-dropped abort (#5) and the no-match lobby exit (#6).Testing
node tests/pipeline.test.mjs— 34 checks, no dependencies, no browser. The module is extracted from the experiment file and run against a fake DataPipe that distinguishes attempts from what actually lands at OSF, since a failed POST leaves no file behind.Covers chunking and disjointness, every response class, requeue-on-failure, concurrent saves not double-sending, filename uniqueness and fallbacks, the redirect budget, idempotent replay, inert behaviour when unconfigured, and end-to-end reassembly of a partial lossy session.
Not browser-tested end to end — that needs a live experiment ID.
For the analysis side
Chunks reassemble by concatenating
trialsacross a dyad's files; the two participants join ondyad_id.Each chunk carries
row_ranges: a list of inclusive[start, end]pairs, not a single pair. A chunk may carry rows reclaimed from an earlier failed save alongside its new ones, and those are not contiguous with each other. The invariant to check is that the union of allrow_rangesfor a participant covers0..Nexactly once — no gaps, no overlaps.chunk_seqmay legitimately gap, since a save that fails outright still consumes a sequence number, so it is not a completeness check.Before this does anything
CONFIG.DATAPIPE_EXPERIMENT_IDis empty, so saving is inert and warns to the console. Deliberate — safer than a placeholder that uploads to an unintended project. It needs #15, which is now the blocking step for seeing any of this work for real.Known follow-ups
dyad_idandSEEDboth read?mp_session=. Firebase swap silently breaks SEED — every dyad would share one trial order #9 re-derives that from the Firebase room id; it should follow automatically, but verify it does — adyad_idthat differs between partners silently unjoins every dyad in the dataset.DECISIONS.mdare open IRB questions on retention and researcher review, and they now have a concrete destination to be answered about.🤖 Generated with Claude Code
Review follow-ups (
5ad35d0)The blocking bug was real and is fixed.
savedThrough -= rows.lengthassumed saves fail in LIFO order. Reproduced exactly as described: rows 0-1 lost permanently, 3-4 duplicated, ranges overlapping. The compounding problem was the silence — those overlapping ranges break the very contiguity invariant this PR told the analysis side to check, so the detection mechanism stops holding precisely when it is needed.savedThroughis now a high-water mark that only advances. Failed saves push their[start, end]ranges onto apendinglist, and each save claims everything pending plus everything new. That is why chunks now emitrow_ranges(a list) rather thanrow_range(a pair) — a chunk may carry reclaimed rows that are not contiguous with its new ones. The analysis section above is updated accordingly.The regression test is included. It is exactly the case the old concurrency test missed, because both of its saves succeeded.
classify()now accepts any 2xx. Agreed — treating an unexpected 200 as failure would requeue rows that landed and duplicate them.configured()uses truthiness; the test's extraction regex now fails with a readable message.dyadKey()is documented as defensive-only, pointing atDYAD_ID's own fallback added on the base branch inc2c69c2.Tests: 39, up from 34.