diff --git a/AGENTS.md b/AGENTS.md index 3608c738c3..af813dde37 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -4,6 +4,8 @@ Single entry point for all agent-driven and AI-coding work in this repository. S > **Opening, updating, or merging pull requests is never an agent's job.** Do the work on a branch and push it when asked; the human opens and manages the PR. Also confirm a branch still exists before pushing to it — pushing to a deleted (e.g. post-merge) branch silently recreates it. +> **Debugging a failed Terra/WDL pipeline test?** See [FISS_MCP_RUNBOOK.md](FISS_MCP_RUNBOOK.md) — submission URL → the exact failing task/metric, via the `fiss-mcp` tools (subworkflow log drill-down, GCS traversal, stale-run checks). + ## Authoritative References | Topic | Document | @@ -136,7 +138,7 @@ A WARP "Plumbing" or "Scientific" test is a **CI-integrated test that runs the p 1. **Test input JSON(s)** — `pipelines/wdl//test_inputs/{Plumbing,Scientific}/.json`, keyed by the **pipeline's** input names (`.foo`). Host the input data under `gs://pd-test-storage-public//input/{plumbing,scientific}/...`. Plumbing = tiny/fast/cheap (every PR); Scientific = realistic end-to-end (PRs to master, or on demand). 2. **Test wrapper** — `verification/test-wdls/Test.wdl`: imports the pipeline, `Verify`, `Utilities`, and `TerraCopyFilesFromCloudToCloud`; declares **every pipeline input a test JSON might set** plus the framework-injected `truth_path`, `results_path`, `update_truth` (and `cloud_provider` if the pipeline takes it); sets `meta { allowNestedInputs: true }`. It calls the pipeline, gathers outputs into an `Array[String]`, copies them to `results_path`, copies to `truth_path` when `update_truth`, else runs `GetValidationInputs` and calls `Verify`. **Forward every testable input** — an input the pipeline has but the wrapper omits crashes the moment a test JSON sets it (see *Test inputs* above). -3. **Verification** — `verification/Verify.wdl` compares each output to truth (tolerantly). Keep pipeline-specific compare tasks **here, not in the shared `verification/VerifyTasks.wdl`**, so editing them doesn't trip every other pipeline's path filter. When a `Verify*` comparison fails on tiny float or "1-of-N" differences, it is usually known run-to-run nondeterminism, not a regression — compare with a tolerance by extending an existing tolerant task (e.g. `CompareGeneMetricsWithTolerance`, or the tolerant `CompareSAMs` in `VerifyRNAWithUMIs.wdl`) rather than adding a strict check. +3. **Verification** — `verification/Verify.wdl` compares each output to truth (tolerantly). Keep pipeline-specific compare tasks **here, not in the shared `verification/VerifyTasks.wdl`**, so editing them doesn't trip every other pipeline's path filter. When a `Verify*` comparison fails on tiny float or "1-of-N" differences, it is usually known run-to-run nondeterminism, not a regression — compare with a tolerance by extending an existing tolerant task (e.g. `CompareGeneMetricsWithTolerance`, or the tolerant `CompareSAMs` in `VerifyRNAWithUMIs.wdl`) rather than adding a strict check. **Setting a nondeterminism tolerance:** once a human contributor confirms an observed drift is acceptable (not a real regression), set the threshold to **double the observed drift**, not just above it. A threshold tuned to barely clear one run's drift (≤2×) flakes intermittently, because the drift varies run-to-run; 2× buys margin while staying orders of magnitude below any real regression, so you lose no sensitivity. Any metric absent from a percentage-threshold table (e.g. `CompareAtacLibraryMetrics` in `VerifyTasks.wdl`) defaults to **exact match** — a newly-nondeterministic metric must be added explicitly or it fails on the first sub-unit drift. 4. **GitHub Actions entry point ("the buttons")** — `.github/workflows/test_.yml`: `on: pull_request` with a `paths:` filter scoped to the pipeline's files (pipeline dir, its tasks, `Verify.wdl`, `Test.wdl`, `TerraCopyFilesFromCloudToCloud.wdl`, the two workflow files, `firecloud_api.py`) **and** `workflow_dispatch` with inputs `useCallCache`, `updateTruth`, `testType` (choice Plumbing/Scientific), `truthBranch`. The job `uses: ./.github/workflows/warp_test_workflow.yml` with `pipeline_name: Test`, `dockstore_pipeline_name: `, `pipeline_dir`, those inputs, and `secrets: { PDT_TESTER_SA_B64, DOCKSTORE_TOKEN }`; `permissions: { contents: read, id-token: write, actions: write }`. (`warp_test_workflow.yml` is the shared reusable workflow: it creates the Terra method config, submits, polls, copies results, and runs verification when not updating truth.) 5. **Dockstore registration *and publish*** — add a `Test` entry (`name`, `subclass: WDL`, `primaryDescriptorPath: /verification/test-wdls/Test.wdl`) to `.dockstore.yml` (the pipeline itself is usually already registered). Then **publish it in the Dockstore UI** — a manual step that's easy to forget and not caught by anything in-repo: wait ~5 min for Dockstore to ingest the new descriptor, go to the [Dockstore dashboard](https://dockstore.org/dashboard), find `warp/Test` via the *Search Workflows* field, then **Versions → Actions → Set as Default Version → Publish**. The CI resolves the workflow by `dockstore_pipeline_name`, so an unpublished / no-default-version workflow makes the Terra method-config step fail. 6. **Seed truth FIRST** — a brand-new test has no golden files. Run the Actions workflow manually (`workflow_dispatch`) with `updateTruth: true` and the right `truthBranch` to populate the truth bucket; only then do compare-runs pass. A compare-run before truth exists fails with nothing to diff. The truth path is keyed by the **test JSON's filename**, not `input_id`: `gs://pd-test-storage-public//truth////` (e.g. `mouse_v4_snRNA_example.json` → `.../mouse_v4_snRNA_example/`). So changing only `input_id` overwrites the *same* truth key; renaming the JSON file starts a fresh one. diff --git a/FISS_MCP_RUNBOOK.md b/FISS_MCP_RUNBOOK.md new file mode 100644 index 0000000000..6a27bc1418 --- /dev/null +++ b/FISS_MCP_RUNBOOK.md @@ -0,0 +1,77 @@ +# FISS-MCP runbook — debugging a failed WARP Terra test + +**Scope:** debugging WARP's **Terra-based WDL pipeline tests** (the Plumbing/Scientific CI +tests that run a `Test` workflow on Terra and compare outputs to truth). Nothing +else. It assumes the `fiss-mcp` tools are available. Goal: go from a Terra submission URL to +the exact line that failed, fast, without fumbling. + +## 0. Parse the URL + +``` +https://app.terra.bio/#workspaces///submission_history/ +``` +e.g. `warp-pipelines / WARP Tests / 22618e75-...`. Note `%20` = space in the workspace name ("WARP Tests"). + +## 1. Submission → which workflow failed + +`get_submission_status(namespace, workspace, submission_id)` +- `status_summary` tells you Failed/Succeeded counts. +- Grab each `workflows[].workflowId`. A submission usually has ONE workflow (the `Test` wrapper). +- If `status: Succeeded` you're done — that URL is green, stop. + +## 2. Workflow → which task failed (summary mode ALWAYS first) + +`get_job_metadata(..., workflow_id, mode="summary")` +- Cheap (~1-2K tokens). Returns `failed_tasks[]` and `workflow_failures[].causedBy` chain. +- The `causedBy` message names the real culprit, e.g. + `Job VerifyMultiome.CompareAtacLibraryMetrics:NA:1 exited with return code 1`. +- **Gotcha:** the top-level failed task is often `Test.Verify` — that is a *sub-workflow*, not the real failure. Its `stderr_url`/`stdout_url` are empty at the parent level. The real task lives one level down (`VerifyMultiome.`). Go to step 3. + +## 3. Find the real task's logs (subworkflow drill-down via GCS) + +`get_job_metadata` extract mode CANNOT path into subworkflows here — call keys contain dots +(`TestMultiome.Verify`) and the dot-path parser chokes (`Key 'TestMultiome' not found`). **Don't fight it.** Walk the GCS tree instead with `list_gcs_objects(recursive=false)`: + +``` +gs:///submissions/intermediates////call-Verify/ + └─ / (e.g. VerifyMultiome/) + └─ / (a fresh UUID, different from WF_ID) + └─ call-/ (e.g. call-CompareAtacLibraryMetrics/) + ├─ stdout ← comparison output usually goes HERE + ├─ stderr ← often 0 bytes; don't assume the error is here + ├─ rc ← 2-byte file: the return code + └─ script ← the exact bash that ran, if you need it +``` +Get the bucket from any `*_url` in step 2 (`gs://fc-...`). List each level with `recursive=false` and read the `prefixes[]` to descend one hop at a time. + +## 4. Read the log + +`read_gcs_object(gcs_uri, offset, max_bytes)`. + +Prefer `stdout` over `stderr` for WARP verification tasks — the python comparison scripts +print PASS/FAIL to stdout; stderr is frequently empty. For a comparison task, the lines you +want are `Error: Metric ... exceeds threshold`. + +**Sandboxed-agent caveat (does NOT apply to a human with gsutil):** in the agent container, +`read_gcs_object` returns an opaque `<>` reference for large reads (a context- +compression layer you can't actually read), and it *dedups identical re-reads* to the same +ref. So: keep `max_bytes` small (≤ ~1400 → real UTF-8 text), and walk the file in +**contiguous non-overlapping windows** (`offset` 0, 400, 1800, 2500, …) — never re-read the +same range; if a window comes back compressed, shift the offset instead of retrying it. + +## 5. Other gotchas + +- **No `gsutil`/`gcloud` in the agent container.** Use the MCP GCS tools (`list_gcs_objects`, `read_gcs_object`, `get_gcs_object_metadata`), not shell. (A human on a laptop can just `gsutil cat` the stdout path from step 3 and skip step 4's windowing entirely.) +- `get_workflow_logs(fetch_content=false)` gives you all task stderr/stdout URLs in one shot — handy for the *parent* tasks, but it won't reach into subworkflows (their entries have empty URLs). Use step 3 for those. +- For infra-looking failures (RC 137 / "stopped before command finished" / 0-second tasks) use `get_batch_job_status` — those errors are NOT in the GCS stderr. +- `cost` in the status is real money spent; a green submission still cost a few cents. +- A **stale** failure is common: confirm the submission's date against the branch HEAD. A run from before a fix landed will show the old failure. + +## 6. One-screen cheat sheet + +``` +get_submission_status → workflowId, Failed? +get_job_metadata mode=summary → failed task + causedBy (spot the .Verify subworkflow) +list_gcs_objects recursive=false → descend call-Verify///call-/ +read_gcs_object → read stdout (human: gsutil cat; agent: small windows, see §4) +``` diff --git a/verification/VerifyTasks.wdl b/verification/VerifyTasks.wdl index 5d9aaa507f..e7c85daeac 100644 --- a/verification/VerifyTasks.wdl +++ b/verification/VerifyTasks.wdl @@ -222,27 +222,28 @@ import hashlib thresholds = { "sequenced_reads": 0.0000001, # ~61 reads on a 614M-read library; raised from 6.6e-9 (allowed 4) which flaked on an observed 9-read drift "fraction_Q30_bases_in_read_1": 0.0000000054, - "fraction of high-quality fragments in cells": 0.000000054, - "fraction_of_transposition_events_in_peaks_in_cells": 0.00000037, - "fraction_duplicates": 0.00000017, + "fraction of high-quality fragments in cells": 0.00022, + "fraction_of_transposition_events_in_peaks_in_cells": 0.00006, + "fraction_duplicates": 0.00000105, + "fraction_fragment_in_nucleosome_free_region": 0.0000027, + "fraction_fragment_flanking_single_nucleosome": 0.0000012, + "fraction_of_high-quality_fragments_overlapping_tss": 0.00000087, + "number_of_peaks": 0.00003, + "fraction_of_genome_in_peaks": 0.0000069, + "mean_raw_read_pairs_per_cell": 0.00088, + "median_high-quality_fragments_per_cell": 0.000087, + "atac_percent_target": 0.001, + "number_of_cells": 0.00088, "fraction_confidently_mapped": 0.000000123, - "fraction_unmapped": 0.0000016, + "fraction_unmapped": 0.000014, "fraction_nonnuclear": 0.00000079, - "fraction_fragment_in_nucleosome_free_region": 0.00000059, - "fraction_fragment_flanking_single_nucleosome": 0.00000057, "tss_enrichment_score": 0.0000024, - "fraction_of_high-quality_fragments_overlapping_tss": 0.00000025, - "number_of_peaks": 0.0000074, - "fraction_of_genome_in_peaks": 0.0000024, - "fraction_of_high-quality_fragments_overlapping_peaks": 0.00000030 + "fraction_of_high-quality_fragments_overlapping_peaks": 0.0000015 } thresholds = {k.lower(): v for k, v in thresholds.items()} - def calculate_md5(file_path): - """Calculates the MD5 checksum for a file.""" - print(f"Processing file: {file_path}") hash_md5 = hashlib.md5() with open(file_path, "rb") as f: for chunk in iter(lambda: f.read(4096), b""): @@ -752,13 +753,16 @@ task CompareH5adFilesGEX { print("Doublet score is allowed to be different") elif x.startswith("emptydrops_"): # EmptyDrops is Monte-Carlo stochastic; the WARP nondeterminism catalog - # allows these columns to vary within 1%. Gate on the column-sum rel diff. + # allows these columns to vary. Gate on the column-sum rel diff at 2x the + # observed drift (emptydrops_PValue drifted 1.13%; see AGENTS.md — set a + # nondeterminism tolerance to double the human-accepted drift). + emptydrops_tol = 0.023 denom = abs(y.sum()) if y.sum() != 0 else 1 rel = abs(z.sum() - y.sum()) / denom - if rel <= 0.01: - print("%s column sums within 1%% tolerance (rel diff %.4f%%); allowed" % (x, rel*100)) + if rel <= emptydrops_tol: + print("%s column sums within %.1f%% tolerance (rel diff %.4f%%); allowed" % (x, emptydrops_tol*100, rel*100)) else: - exit("Cell Metric %s sums differ by %.4f%%, exceeds 1%% tolerance" % (x, rel*100)) + exit("Cell Metric %s sums differ by %.4f%%, exceeds %.1f%% tolerance" % (x, rel*100, emptydrops_tol*100)) else: exit("Cell Metric does not match") print("Comparing test gene metrics to truth gene metrics using truth as ref")