refactor: decompose benchmark report into 4-step pipeline - #102
Conversation
… API Replaces tw runs dump CLI. Fetches workflow, metrics, tasks, progress endpoints directly. Handles workspace ID resolution and pagination.
Self-contained HTML report with dark theme. Sections: benchmark overview, run overview, process box plots, task scatter, cost breakdown. Replaces R/Quarto/renv stack entirely.
- Add nf-boost plugin for request/fromJson/toJson
- ids.map { fetchRunData } replaces tw runs dump for benchmarks
- generate_benchmark_report uses v2, generate_benchmark_report_legacy for old R path
- Existing SEQERA_RUNS_DUMP + MULTIQC flow unchanged
- read_json_auto needs file path not string — write tmp files - Replace fetchdf().to_dict() with fetch_dicts() helper (no numpy/pandas) - Tested against scheduler benchmarking tw dump data
- Move brand.yml to assets/ where report assets live - Add load_brand() to parse brand.yml color definitions - Replace hardcoded #4256e7 accent with deep_green (#087F68) - Replace hardcoded COLORS palette with brand-derived palette - Add --brand CLI flag, pyyaml dep, brand_yml process input - Update nf-test to pass brand.yml
- Add seqera_logo_color.svg from seqeralabs/logos repo - Replace hardcoded fake SVG with real logo via --logo flag - Add logo_svg input to NF process and wire through workflow/test
- add explicit do_not for white text on Brand Green - add deep green size constraint for body text - remove duplicate brand_deep entry - add white text/graphic roles on dark backgrounds
Pass all 7 brand tokens to template. Replace ~70 hardcoded hex colors in CSS and chart configs with Jinja brand variables. Zero hardcoded colors remain in the template.
Google Fonts CDN for Inter (body + headings). Heading weights bumped to 500-600 to approximate Degular feel.
Brand-aligned theme JSON covering color palette, axes, title, legend, tooltip, dataZoom, boxplot, scatter, gauge, and all axis types. Colors sourced from brand.yml.
Charts now init with 'seqera' theme. Removes per-chart backgroundColor, axisLabel.color, title.textStyle, and explicit series color overrides.
Old palette had blue, orange, purple, pink — not in brand spec. New 8-color palette uses brand greens at varying lightness/saturation plus two neutrals (Medium Grey, dark grey) for contrast.
8 inline SVG symbol sprites: dashboard grid, play/run, table, bar chart, workflow nodes, checklist, server stack, scatter. Icons in brand accent color on headings, inherit color in nav. Side nav uses flex layout for icon+text alignment.
Agent-facing documentation covering architecture, data flow, module responsibilities, brand system, and gotchas. Derived from session history across the v2 redesign and brand work.
The v2 Python+DuckDB+eCharts report fully replaces this. Removes: main.nf, nextflow.config, AGENTS.md, tests/
- Drop BENCHMARK_REPORT include and generate_benchmark_report_legacy block - Remove generate_benchmark_report_legacy and remove_failed_tasks params - Remove legacy module includeConfig - Clean remove_failed_tasks from schema
- Root AGENTS.md: single architecture, no v1/v2 split - modules/local/AGENTS.md: drop benchmark_report row - workflows/AGENTS.md: drop legacy execution path - docs/DESIGN.md: remove migration section, fix process names
v1 is gone, drop the _v2 suffix everywhere: module dir, process name, includes, docs, tests.
API returns tasks as [{task: {...}}, ...] not flat dicts.
build_database doesn't unwrap these yet, so cost/hash fields are None.
Tests marked xfail(strict=True) — will fail CI if bug silently fixes itself.
Task hash 'ab/cdef12' (Nextflow workdir format with '/') never matches CUR hash 'abcdef12' because LEFT(t.hash, 8) keeps the slash. xfail(strict=True) on both cost tests.
LEFT(REPLACE(t.hash, '/', ''), 8) so Nextflow workdir hashes like 'ab/cdef12' match CUR MD5 prefixes like 'abcdef12'.
Validates build_database + query_run_costs against committed SD-1043 data (cpu + g5 runs). Catches regressions in nested-task unwrap, cost population, and group assignment.
Hardcoded grid.left: 250 wasted space for short labels (cpu, g5, etc), squeezing chart area and causing x-axis label overlap and clipped axis names. containLabel lets eCharts auto-size margins dynamically.
- Grid: add containLabel + percentage margins as theme defaults - Font: Inter with muted #666 axis labels/ticks (was brand green) - Tooltip: borderRadius 4, lighter bg, better padding - Legend: circle icons, muted text, animation - Bar: borderRadius 2, emphasis focus series, labelLayout hideOverlap - Split lines: subtle #E8E8E8 (was #CFD0D1) - Animation: 500ms duration - Color palette: diverse hues (deep green, blue, dark, gold, teal, mauve) replacing monotone all-greens for better series distinction
- Palette: diverse hues matching theme (was all-greens) - hbarChart/hbarStacked: use theme grid defaults, add bar borderRadius and emphasis focus, slightly taller per-bar spacing (45px) - Workflow status: semantic green (#16a34a) / red (#dc2626) - Process/boxplot/instance/scatter: containLabel replaces hardcoded left margins (350/250/180/70px) - Chart row gap: 32px (was 20px) for breathing room
Drop fontSize: 9/10 from process, boxplot, instance usage axis labels and legend - let theme defaults (11px) apply consistently. Remove barMaxWidth: 16 from process cost bars - theme default (60) is better. Add borderRadius + emphasis to process cost series.
Switch all xAxis from nameLocation:'end' to 'center' with nameGap:25
to prevent label text from being clipped at the grid boundary.
- hbarChart: nameLocation 'end'→'center', nameGap 8→25
- hbarStacked: add opts parameter with xName support
- Workflow Status: pass { xName: 'Tasks' } so axis is labeled
- Process runtime/cost, instance usage, task scatter, boxplot:
add nameLocation:'center' and nameGap:25 to all standalone axes
- Task scatter yAxis: nameGap:40 for longer label
Apply Tufte's direct-labeling principle: when there are few series, label marks directly instead of forcing legend-bouncing. - hbarChart: add series extraction + conditional label for 2–3 series - hbarStacked: inside segment labels (white text), hide legend when ≤3 - Process scatter: markPoint on max-x data point per group - Process cost bars: right-side group labels, conditional legend - Task scatter: markPoint labels per group, conditional legend - Single-series bars left unchanged (y-axis already labels groups) - Stacked bar labels hidden when segment value ≤5 (prevent overflow)
Add auto-generated takeaway subtitles to all charts following Tufte's principle that titles should state a finding, not just label variables. - Add takeaway() helper: compares groups and generates one-line insight (e.g. 'g5 was 3.4× cheaper ($5.4 vs $18.3)') - hbarChart/hbarStacked: accept opts.subtitle, render via title.subtext - Grid top adjusts dynamically (60px when subtitle present, 40px otherwise) - Wall time: 'X was N× faster' - CPU time: 'X was N× lower' - Compute cost: 'X was N× cheaper' - Workflow status: 'All tasks succeeded' or 'N tasks failed across all runs' - CPU/Memory efficiency: 'X was N× more efficient' - Data read/written: 'X was N× less I/O' - Process cost chart: markLine annotation on most expensive process
lib/ classes compile before plugin classes are on the classpath, so importing nextflow.boost.BoostFunctions always fails at semantic analysis. Use java.net.URL directly instead.
Was commented out but its output was still referenced, causing MissingPropertyException at runtime.
Missing closing brace for publish_dir_mode property caused JSON parse error during nf-schema validation.
Decompose the JSON loading and normalization from the monolithic
benchmark_report.py into a standalone script.
- Extracts runs, tasks, and metrics from SeqeraApi JSON into CSVs
- Properly extracts cachedCount from workflow.stats
- Keeps COMPLETED + CACHED tasks, drops FAILED (configurable)
- Handles both nested ({task: {...}}) and flat task formats
- Adds Nextflow process module (CLEAN_JSON)
- 16 tests covering cached extraction, nested tasks, filtering, CSV export
Separate CUR parquet processing from the monolithic benchmark_report.py. - Auto-detects CUR 2.0 (MAP format) vs CUR 1.0 (flattened columns) - Handles presence/absence of nf_unique_run_id column - Outputs normalized costs.csv: run_id, process, hash, cost, used/unused - Adds Nextflow process module (CLEAN_CUR) - 8 tests covering both CUR formats
Extract all DuckDB query logic from benchmark_report.py into a standalone script that reads CSVs and outputs query result JSONs. - 9 query functions: benchmark_overview, run_summary, run_metrics, run_costs, process_stats, task_instance_usage, task_table, task_scatter, cost_overview - run_summary now includes cachedCount - task_table and task_scatter include CACHED status tasks - CUR hash join normalizes task hash (strips '/') before comparing - Adds Nextflow process module (BUILD_TABLES) - 5 tests covering cached count, task table, CUR join
Extract the Jinja2/eCharts rendering into a standalone script that reads pre-computed JSON files. Does NO DuckDB queries. Cached task display: - 'Tasks cached' column in run summary table - Workflow status chart shows succeeded/failed/cached stacked bars (amber #f59e0b for cached, only when cachedCount > 0) - Status subtitle dynamically includes cached task count - Adds Nextflow process module (RENDER_REPORT) - 8 tests covering rendering, cached display, brand loading
Replace monolithic BENCHMARK_REPORT process with the decomposed pipeline:
API (head job) → CLEAN_JSON → BUILD_TABLES → RENDER_REPORT
CLEAN_CUR ↗ (optional)
- Step 0: API fetching unchanged (head job, nf-boost map{})
- Step 1: CLEAN_JSON — raw JSON → normalized CSVs
- Step 2: CLEAN_CUR — AWS CUR parquet → costs CSV (optional)
- Step 3: BUILD_TABLES — DuckDB queries → result JSONs
- Step 4: RENDER_REPORT — pre-computed data → HTML
Update docs/DESIGN.md with new architecture diagram and local
testing instructions. Update AGENTS.md rebuild commands.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5807555d48
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| script: | ||
| """ | ||
| clean_json.py \\ |
There was a problem hiding this comment.
This process invokes clean_json.py directly, but the new scripts added in this commit (bin/clean_json.py, bin/clean_cur.py, bin/build_tables.py, bin/render_report.py) were committed with non-executable mode (100644), so shell execution returns Permission denied (exit 126) when generate_benchmark_report is enabled. Please either set executable bits on these scripts or invoke them via python ... in the module scripts.
Useful? React with 👍 / 👎.
| CLEAN_JSON.out.metrics_csv.ifEmpty(file("${projectDir}/assets/NO_FILE")), | ||
| ch_costs_csv.ifEmpty(file("${projectDir}/assets/NO_FILE")), |
There was a problem hiding this comment.
These fallbacks pass ${projectDir}/assets/NO_FILE into BUILD_TABLES when optional inputs are absent, but there is no assets/NO_FILE file in the repo and BUILD_TABLES declares both inputs as required paths. In runs without metrics and/or without CUR, this path fallback can make the task fail during input staging instead of simply skipping the corresponding flag.
Useful? React with 👍 / 👎.
Refactor benchmark pipeline UI
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ca955e61c6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| SEQERA_RUNS_DUMP( | ||
| ids_split.fetch_run_dumps, | ||
| ids, | ||
| seqera_api_endpoint, |
There was a problem hiding this comment.
Passing the full ids channel directly into SEQERA_RUNS_DUMP removes the previous workspace == 'external' bypass, so rows that provide pre-exported logs are now treated as live API/CLI fetches. Inputs that still match assets/schema_input.json (workspace=external plus logs) will fail because this path invokes workspace resolution and tw runs dump against external instead of reusing the provided dump path.
Useful? React with 👍 / 👎.
|
|
||
| conda 'python=3.12 duckdb=1.3 typer=0.15 pyyaml=6' |
There was a problem hiding this comment.
The new benchmark steps are conda-only, but this pipeline’s standard container profiles disable conda (docker/singularity set conda.enabled = false in nextflow.config). When --generate_benchmark_report is enabled, these tasks run without duckdb/typer/jinja2 in those profiles and fail at runtime. Please add container images (or equivalent profile-safe env provisioning) for the new report processes.
Useful? React with 👍 / 👎.
| RENDER_REPORT( | ||
| BUILD_TABLES.out.tables_dir, | ||
| file("${projectDir}/assets/brand.yml", checkIfExists: true), | ||
| file("${projectDir}/assets/seqera_logo_color.svg", checkIfExists: true), | ||
| ) |
There was a problem hiding this comment.
The workflow invokes RENDER_REPORT but does not route its html output anywhere, and unlike the legacy benchmark module there is no publish configuration attached to this new path. In practice, benchmark_report.html stays in task work directories instead of being surfaced under --outdir, so enabling benchmark reporting does not produce a user-visible report artifact.
Useful? React with 👍 / 👎.
2 participants
Summary
Decomposes the monolithic
BENCHMARK_REPORTprocess into a 4-step pipeline:New processes
CLEAN_JSONbin/clean_json.pyruns.csv,tasks.csv,metrics.csvCLEAN_CURbin/clean_cur.pycosts.csv(auto-detects CUR 1.0 vs 2.0)BUILD_TABLESbin/build_tables.pyRENDER_REPORTbin/render_report.pyCached task support (new)
cachedCountextracted fromworkflow.stats.cachedCountinto run summaryTests
37 new tests across 4 test files + 16 legacy tests still passing (53 total).
Stacked on #