Skip to content

Sign Parabricks skill - #51

Open
ohadmo wants to merge 17 commits into
mainfrom
parabricks-skill
Open

ohadmo wants to merge 17 commits into
mainfrom
parabricks-skill

Conversation

@ohadmo

@ohadmo ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

@greptileai

@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 1/5

[Medium impact] Updates documentation and test data for genomics skill.

The PR is not safe to merge while plugin synchronization fails and the PON and readiness guidance regressions remain.

Findings

  1. P1 Nested payload fails sync ▶
  2. P1 Probe timeout is ignored ▶
  3. P1 Translator fails on Python 3.7 ▶
  4. P2 Evaluation metadata conflicts ▶
  5. P2 Neutral verdict missing from card ▶
  6. P2 Neutral verdict cites missing decline ▶

Summary

The PR refreshes Parabricks command references and evaluations, replaces the Python readiness helper with Bash, and updates publication artifacts. The generated plugin payload is not synchronized with its source, the revised PON guidance conflicts with the supported linear-reference workflow, and the Bash helper can lose its timeout guarantee.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart LR
  S[Source Parabricks skill] --> C[Plugin sync]
  C --> P[Flat distributed skill]
  P --> A[Card, benchmark and signature]
  S --> R[Tool references and readiness helper]
Loading

Reviews (17) · Last reviewed commit: "Merge remote-tracking branch 'origin/par..." · Reviewed by Greptile

Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Comment thread skills/bionemo-agent-toolkit/skills/parabricks/skill-card.md Outdated
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

Comment thread skills/bionemo-agent-toolkit/skills/parabricks/SKILL.md
Comment thread library-skills/parabricks/SKILL.md Outdated
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Comment thread skills/bionemo-agent-toolkit/skills/parabricks/BENCHMARK.md
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

Comment thread library-skills/parabricks/evals/evals.json Outdated
Comment thread library-skills/parabricks/references/runtime-environment.md
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Comment thread skills/bionemo-agent-toolkit/skills/parabricks/skill-card.md Outdated
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

Comment on lines +69 to +72
## Evaluation Results: <br>
| Measure | Claude Code (Baseline → Skill Uplift) | Codex (Baseline → Skill Uplift) |
|---|---:|---:|
| Overall | 97.1% | 92.6% |

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 Neutral verdict missing from card

The refreshed card lists strong evaluation scores but omits the benchmark’s new NEUTRAL publication verdict and its recommendation to collect more evidence before deciding to publish. Someone using the card as a release summary could mistake the scores for a positive publication recommendation. Please include the verdict or link to the benchmark alongside the results.

Knowledge Base Used: Skill quality and compliance controls

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!

Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

Comment thread skills/bionemo-agent-toolkit/skills/parabricks/skill.oms.sig Outdated
svc-nvskills-signing and others added 2 commits October 2, 2026 19:07
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 2, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

svc-nvskills-signing and others added 2 commits October 2, 2026 22:22
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
Signed-off-by: Ohad Mosafi <omosafi@nvidia.com>
@ohadmo

ohadmo commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

/nvskills-ci

result["issues"].append("Parabricks will generate read groups. Confirm sample metadata if read-group identity matters; no metadata was inferred by this helper.")
result["notes"].append("Draft only: paths, reference/index compatibility, numeric ranges, runtime readiness, and biological output parity are not validated.")
result.update(status="needs_review" if result["issues"] else "draft",
draft_argv=argv, command=shlex.join(argv))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Translator fails on Python 3.7

If the host uses Python 3.7, every otherwise valid translation reaches shlex.join, which is unavailable in that version. The helper then raises an uncaught AttributeError instead of producing a command, even though its reference says only that Python 3 is required. Specify Python 3.8 or newer for this helper, or use a compatible command formatter.

Comment thread skills/bionemo-agent-toolkit/skills/parabricks/skill-card.md Outdated
Comment thread skills/bionemo-agent-toolkit/skills/parabricks/BENCHMARK.md
Signed-off-by: nvskills-svc-account <svc-nvskills-signing@nvidia.com>
@@ -0,0 +1,153 @@
# Skill Benchmark: parabricks

> **Overall verdict: NEUTRAL — One or more dimensions remain below PASS**

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 Neutral verdict cites missing decline

The refreshed benchmark says its NEUTRAL verdict is because one or more dimensions are below PASS. But every reported Codex dimension exceeds the report’s 50% PASS threshold, and every Claude Code score is unavailable. This explanation points readers to a low score the results do not show, making it harder to tell whether the limitation is missing evaluation evidence instead.

Knowledge Base Used: Skill quality and compliance controls

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!

- Update tool references and command conventions for Parabricks 4.7.1
- Improve planning triggers, option translation, and input safeguards
- Replace the Python readiness helper and adapt tests and eval fixtures

Signed-off-by: Angel Pizarro <apizarro@nvidia.com>
…s-skill

Signed-off-by: Angel Pizarro <apizarro@nvidia.com>
@@ -0,0 +1,218 @@
---

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Nested payload fails sync

The new parabricks/parabricks/ directory exists in the generated plugin payload but not in the source skill. The plugin freshness check treats its files as extra content and fails. The top-level distributed SKILL.md also remains at version 1.2.1 while the source is 1.2.4, so users receive older guidance. Regenerate the flat payload from the source instead of adding a second skill directory inside it.

Knowledge Base Used: Plugin synchronization and generated skills

Comment on lines +122 to +124
else
"$@" >"$TMP_DIR/out" 2>"$TMP_DIR/err" </dev/null
CMD_RC=$?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Probe timeout is ignored

If neither timeout nor gtimeout is installed, this fallback runs external probes without a time limit. A stalled Docker command can therefore leave the readiness check hanging indefinitely even when the caller supplied --timeout, rather than returning a report.

Knowledge Base Used: Genomic acceleration library integrations

Comment on lines +13 to +14
- Tasks: 18 evaluation tasks (17 positive, 1 negative)
- Dataset digest: `sha256:6e9efe466a1409e4cbcd58924a921028b12d3af7cce343c86b975cfbf78f9f1b` (skill-evaluator-dataset-snapshot/1)

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 Evaluation metadata conflicts

The refreshed results cover seven live tasks, but this metadata still says 18 tasks and gives a different dataset digest from the seven-task skill card. Readers cannot tell which dataset produced the published scores and verdict, making the release evidence harder to assess.

Knowledge Base Used: Skill quality and compliance controls

@greptile-apps

greptile-apps Bot commented Oct 8, 2026

Copy link
Copy Markdown

Comments Outside Diff

These findings could not be posted inline.

  • P1 Linear PON workflow rejected library-skills/parabricks/references/pbrun-prepon.md:70 ▶

    For a standard GRCh38 mutectcaller panel-of-normals workflow, this new guardrail tells agents not to use prepon and the preceding guidance asks for pangenome resources. But the mutectcaller reference requires prepon to prepare the PON resource. The postpon reference likewise forbids standard linear-reference post-processing, so following these instructions can prevent users from preparing and annotating a valid PON workflow.

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.

3 participants