Skip to content

SDK toolchain upgrade + raw-file download, configurable scan limit, aligned randomize#1

Merged
mizraelson merged 5 commits into
mainfrom
chore/sdk-upgrade-silicon-docker
Jul 10, 2026
Merged

SDK toolchain upgrade + raw-file download, configurable scan limit, aligned randomize#1
mizraelson merged 5 commits into
mainfrom
chore/sdk-upgrade-silicon-docker

Conversation

@mizraelson

@mizraelson mizraelson commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Summary

Moves the block onto the current SDK toolchain (so the linux/amd64 software image builds on Apple Silicon) and adds several read-inspection improvements plus fixes.

Changes

Toolchain

  • Structurer refresh → package-builder 3.14.1, block-tools 2.12.2, tengo-builder 4.0.15, model/ui 1.79.x. Enables building the software Docker image on arm Macs (3.12 hard-gated docker builds to x64 and skipped them on aarch64).

Features

  • Download raw sample files without a Run — exposed by the pre-run; streams on demand from the backend, never staged to a workdir.
  • Configurable scan limit (default 2,000,000 reads) for the read-numbers/headers/pattern/randomized-range modes; if it exceeds the file's read count the whole file is scanned.
  • Mate-aligned randomize — randomized range keeps paired R1/R2 aligned for both gzipped and uncompressed input (seeded reservoir sampling picks the same ordinals on both sides).
  • Standard SDK "running analysis" overlay during extraction (app-level progress), replacing the custom spinner.

Fixes

  • Block title shows the sample's human-readable label instead of the raw id.
  • Read-header matching tooltip corrected (exact match, not prefix).
  • Removed the incorrect "R1/R2 not mates" warning for randomized gzipped input (pairs are aligned there).

Testing

  • 25 Python unit tests pass (software/tests), including a new test asserting R1/R2 select identical ordinals under randomize.
  • Built via build:dev-remote (amd64 image pushed to public ECR) and exercised on app.research.platforma.bio: raw download without Run, extraction, aligned randomize.

Release

  • Changeset included (minor: model, ui, workflow, software, root block).
  • Intended for the any channel only — do not mark stable.

Greptile Summary

This PR upgrades the SDK toolchain (enabling linux/amd64 Docker builds on Apple Silicon), adds raw-file download without a Run via a new pre-run, makes the read-scan limit configurable, and fixes R1/R2 mate alignment under randomized sampling by replacing byte-offset seeking with unified seeded reservoir sampling for all input types.

  • Unified reservoir sampling (mode_range_random) replaces the split mode_range_random_gzip / mode_range_random_seek paths; the same seed + equal record count guarantees identical ordinal selection on both R1 and R2 sides, fixing the mate-alignment bug. A new test (test_randomize_pairs_aligned) verifies this property.
  • Pre-run download (prerun.tpl.tengo) wraps each sample's original blobs as file.exportFile resources so rawFileExports is populated before any main Run; the model reads them via ctx.prerun.resolve(...) and the UI exposes the download button immediately upon sample selection.
  • scanCap is threaded from BlockDataBlockArgs → workflow params → Python, replacing the previously hardcoded 2_000_000, and a PlNumberField control is shown in the settings panel for all modes that actually scan into the file.

Confidence Score: 4/5

Safe to merge with one notice worth addressing before the next release: the 'randomized over the scanned portion of the file (gzip has no random access)' warning in ReadViewer.vue is factually wrong for uncompressed files and will confuse users who hit the scan cap on plain FASTQ input.

The core algorithmic change — unified reservoir sampling replacing byte-offset seeking — is correct and well-tested by the new mate-alignment test. The pre-run wiring, model changes, and CI updates are clean. The one concrete defect is that the UI warning left over from the gzip-only era now fires for all file types, misleading users of uncompressed files. The forced importHandle cast in ReadViewer is a type-safety concern but unlikely to cause observable misbehaviour given the SDK's usage pattern.

ui/src/pages/ReadViewer.vue — stale warning text and importHandle cast; the approximate notice on line 88–92 needs updating to reflect that sampling is bounded by the scan cap for all file types, not just gzip.

Important Files Changed

Filename Overview
software/src_python/extract_reads.py Unified randomize path — removes byte-offset seek (mode_range_random_seek, _resync_fastq/fasta) and routes all random sampling through reservoir sampling (mode_range_random), fixing R1/R2 mate alignment. The approximate flag is now set for all file types when scan_cap is exceeded, but the UI warning still says "gzip has no random access".
ui/src/pages/ReadViewer.vue Raw-download button moved to pre-run-gated section (no Run required), custom spinner replaced with app-level progress overlay, misleading "gzip has no random access" warning left stale, and importHandle is set via unsafe filename cast.
model/src/index.ts Adds scanCap to BlockData/BlockArgs with NaN-safe clamp, introduces prerunArgs for dataset+sample selection, rewires rawFileExports to read from ctx.prerun instead of direct pcol data, and fixes title to show human-readable label.
workflow/src/prerun.tpl.tengo New pre-run template that wraps each sample's original blobs as downloadable file.exportFile resources (rawFile_R1, rawFile_R2, …) without staging or extraction; for multilane data it picks the first file per readIndex from maps.getKeys iteration order.
workflow/src/main.tpl.tengo Sets the pre-run via wf.setPreRun, and threads args.scanCap through to the extraction tool params (replacing the hardcoded 2_000_000).
software/tests/test_extract_reads.py Adds test_randomize_pairs_aligned (asserts same ordinals across R1/R2 with equal read counts and same seed) and renames seek→reservoir tests; covers the new unified sampling path.
ui/src/pages/SettingsPanel.vue Adds a PlNumberField for scanCap (scan limit), visible only for modes that actually scan into the file; fixes tooltip for header matching (exact, not prefix).
ui/src/app.ts Adds a progress callback driven by isRunning to show the standard SDK 'running analysis' overlay during extraction, replacing the custom spinner removed from ReadViewer.
.github/workflows/build.yaml Migrates CI from the boilerplate runner to the project-specific hz-ubuntu-dind runner, enables tests, adds a release build step, and wires new HZ_CI_* Turbo/cache secrets.

Fix All in Claude Code

Prompt To Fix All With AI
Fix the following 3 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 3
ui/src/pages/ReadViewer.vue:88-92
**Stale "gzip has no random access" warning**

`approximate` is now returned by the unified `mode_range_random` function for **both** gzipped and uncompressed files when the scan cap is exceeded. The existing warning message says *"gzip has no random access"*, which is factually wrong when a user hits the scan limit on a plain `.fastq` file — they'll see incorrect information about why sampling is approximate.

### Issue 2 of 3
ui/src/pages/ReadViewer.vue:182-190
**Redundant disabled guard inside v-if**

The `:disabled="rawFileExports.length === 0"` binding on `PlBtnExportArchive` is dead code: the wrapping `<div v-if="rawFileExports.length > 0">` ensures the component only renders when the array is non-empty, so the disabled condition can never be `true`.

### Issue 3 of 3
ui/src/pages/ReadViewer.vue:149-160
**`importHandle` set to a filename string via unsafe cast**

`FileExportEntry.importHandle` is typed as `ImportFileHandle` (an opaque handle to an imported resource, not a display name). Using `e.fileName as ImportFileHandle` is a forced cast that bypasses the type system. If the SDK uses `importHandle` for anything beyond a display key — e.g. to resolve the blob on the backend — this will silently produce wrong behaviour.

Reviews (1): Last reviewed commit: "feat: standard running overlay and sampl..." | Re-trigger Greptile

Greptile also left 1 inline comment on this PR.

Run `block-tools structure refresh` to move the block onto the current
canonical layout and bump the toolchain (package-builder 3.12.0 -> 3.14.1,
block-tools 2.7.7 -> 2.12.2, tengo-builder -> 4.0.15, model/ui -> 1.79.x).

The main motivation is package-builder 3.14, which builds the linux/amd64
software Docker image on Apple Silicon hosts (3.12 hard-gated docker builds
to x64 and silently skipped them), so the block can be built for a remote
k8s backend from an arm Mac.

Migration fixes required by the refresh:
- add a `typescript` catalog entry (the refreshed block/package.json
  references it via `catalog:`)
- replace the empty placeholder test with a real smoke test (the upgraded
  linter rejects empty / .todo test files)
… randomize

- Raw file download moved to a pre-run (prerun.tpl.tengo) that wraps the
  selected sample's original blobs with file.exportFile. The "Download raw
  files" button is now available as soon as a dataset + sample are chosen,
  with no main Run, and streams on demand without staging to a workdir.
  (The dataset column's data is not reachable from the model, so this must
  be produced workflow-side; passing the raw blob yielded "Key not found
  ctl/file/blobInfo" — exportFile provides the downloadable resource.)
- Scan limit is now a parameter (default 2,000,000 reads), shown for the
  modes that scan; if it exceeds the file's read count the whole file is
  scanned.
- Randomized range keeps paired R1/R2 mate-aligned for both gzipped and
  uncompressed input: both use seeded reservoir sampling, so equal-length
  mates pick identical ordinals. Removed the byte-offset seek path (could
  not preserve alignment) and its dead helpers; added a test asserting two
  mate files select the same ordinals.

Fixes:
- Correct the read-header matching tooltip (exact match, not prefix).
- Drop the incorrect "R1/R2 not mates" warning for randomized gzip.
- Use the SDK app-level `progress` loader (isRunning) so the extraction
  shows the standard "running analysis, no action required" overlay like
  other blocks, instead of a custom in-page spinner. Driven by the main
  Run only, so raw-file pre-run downloads don't trigger it.
- Block title now shows the sample's human-readable label (as picked in
  the dropdown) instead of the raw sample id, falling back to the id when
  the label can't be resolved.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request upgrades the SDK toolchain and introduces several enhancements to the FASTQ reader, including raw file downloads via a pre-run workflow without requiring a full run, a configurable scan limit, and mate-aligned randomized range sampling using seeded reservoir sampling for both compressed and uncompressed inputs. Feedback from the review highlights opportunities for defensive programming, specifically suggesting checks to prevent potential runtime crashes in the pre-run template if dataset data is undefined or keys are empty, as well as checks in the model to avoid TypeErrors if the axes specifications are empty.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment on lines +37 to +48
inputsMap := dataset.data.inputs()
filesByReadIndex := {}
for sKey in maps.getKeys(inputsMap) {
key := json.decode(sKey)
if string(key[0]) != args.sampleId {
continue
}
readIndex := readIndexPos >= 0 ? string(key[readIndexPos]) : "R1"
if is_undefined(filesByReadIndex[readIndex]) {
filesByReadIndex[readIndex] = inputsMap[sKey]
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Defensive programming check:

  1. Since the pre-run executes automatically as soon as a dataset is selected, dataset.data might be undefined if the dataset is still loading or empty. Calling .inputs() on undefined will cause a Tengo runtime crash.
  2. If key is empty, accessing key[0] or key[readIndexPos] will cause out-of-bounds errors. Adding length checks prevents these crashes.
	if is_undefined(dataset.data) {
		return {
			outputs: {},
			exports: {}
		}
	}
	inputsMap := dataset.data.inputs()
	filesByReadIndex := {}
	for sKey in maps.getKeys(inputsMap) {
		key := json.decode(sKey)
		if len(key) == 0 || string(key[0]) != args.sampleId {
			continue
		}
		readIndex := (readIndexPos >= 0 && readIndexPos < len(key)) ? string(key[readIndexPos]) : "R1"
		if is_undefined(filesByReadIndex[readIndex]) {
			filesByReadIndex[readIndex] = inputsMap[sKey]
		}
	}

Comment thread model/src/index.ts
const labels = ctx.findLabels(spec.axesSpec[0]) ?? {};
const indices = spec ? readIndicesFromSpec(spec) : ["R1"];
const ext = spec?.domain?.["pl7.app/fileExtension"] ?? "fastq";
const labels = spec ? (ctx.findLabels(spec.axesSpec[0]) ?? {}) : {};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Defensive programming check: if spec.axesSpec is empty, spec.axesSpec[0] will be undefined, which could cause ctx.findLabels to throw a TypeError. Adding a check for spec.axesSpec[0] ensures robustness.

Suggested change
const labels = spec ? (ctx.findLabels(spec.axesSpec[0]) ?? {}) : {};
const labels = (spec && spec.axesSpec[0]) ? (ctx.findLabels(spec.axesSpec[0]) ?? {}) : {};

Comment thread model/src/index.ts
const spec = ctx.data.inputRef
? ctx.resultPool.getPColumnSpecByRef(ctx.data.inputRef)
: undefined;
const labels = spec ? (ctx.findLabels(spec.axesSpec[0]) ?? {}) : {};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Defensive programming check: if spec.axesSpec is empty, spec.axesSpec[0] will be undefined, which could cause ctx.findLabels to throw a TypeError. Adding a check for spec.axesSpec[0] ensures robustness.

Suggested change
const labels = spec ? (ctx.findLabels(spec.axesSpec[0]) ?? {}) : {};
const labels = (spec && spec.axesSpec[0]) ? (ctx.findLabels(spec.axesSpec[0]) ?? {}) : {};

Comment on lines +182 to 190
<div v-if="rawFileExports.length > 0" class="viewer-controls raw-bar">
<PlBtnExportArchive
:file-exports="rawFileExports"
:disabled="rawFileExports.length === 0"
suggested-file-name="raw-files"
>
Download raw files
</PlBtnExportArchive>
</div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Redundant disabled guard inside v-if

The :disabled="rawFileExports.length === 0" binding on PlBtnExportArchive is dead code: the wrapping <div v-if="rawFileExports.length > 0"> ensures the component only renders when the array is non-empty, so the disabled condition can never be true.

Prompt To Fix With AI
This is a comment left during a code review.
Path: ui/src/pages/ReadViewer.vue
Line: 182-190

Comment:
**Redundant disabled guard inside v-if**

The `:disabled="rawFileExports.length === 0"` binding on `PlBtnExportArchive` is dead code: the wrapping `<div v-if="rawFileExports.length > 0">` ensures the component only renders when the array is non-empty, so the disabled condition can never be `true`.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code

R1 and R2 are separate extraction runs and finish at different times. The
table keyed its columns off the reads resolved so far, so it briefly showed
a single R1 column and then flipped to two when R2 arrived. Drive the column
layout from the dataset's declared read indices instead, and mark a column
still being processed with a "loading" tag.
@mizraelson
mizraelson merged commit 0460a38 into main Jul 10, 2026
11 checks passed
@mizraelson
mizraelson deleted the chore/sdk-upgrade-silicon-docker branch July 10, 2026 20:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant