[bogdankostic]

What does this PR do?

Adds PrimeVul as a NeMo Gum benchmark: paired function-level vulnerability detection over C/C++ code, from Vulnerability Detection with Code Language Models: How Far Are We? (ICSE 2025).

Each task is one function. Every function appears alongside the same function aftr its security fix, and the reported metric is pair-wise correct prediction rate: a pair scores only when both members are classified correctly.

CC @OlegSudakov

Checklist

• I have read the contributing guidelines.
• The change is focused; unrelated "drive-by" edits are tracked as separate issues/PRs.
• Tests added or updated and pass locally, or N/A for docs-only / non-code changes (so CI unit/server checks pass when applicable).
• Pre-commit checks pass locally (pre-commit run --all-files) (so CI lint/format/copyright pass).
• All commits have DCO sign-off (git commit -s) (so the DCO check passes).

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[cmunley1]

/claude review

[claude]

SHIP — LGTM, no reliability concerns.

Reviewed as a correctness-first pass on the verifier, scorer, and paired aggregation. Findings summary:

Verifier/scorer correctness — solid.

• score_verdict requires gold labels (KeyError on missing, tested), binary reward is correct.
• aggregate_paired computes paired accuracy per-repeat then averages, correctly avoiding the pooled "correct k-times-running" metric that would decay with --num-repeats. Pair members align by ROLLOUT_INDEX_KEY_NAME, not input order (tested). Unanswered pairs stay in the denominator, so a model can't inflate its score by declining to answer (tested).
• parse_verdict's tiered matching correctly resists inverting a positive detection whose justification contains the word "no" — the asymmetric-corruption case that would silently damage vulnerable-class recall. Directly tested.

Async / API / deps — clean.

• verify() is async, stateless, no HTTP, no ray.get, no httpx.
• datasets and orjson are declared in pyproject.toml; datasets import is lazy (preparation-time only).
• compute_metrics/get_key_metrics overrides match the AggregateMetricsMixin contract.
• The MANIFEST_FILENAME relocation (registry → benchmarks) is consistent, and excluding manifests from _is_benchmark_config prevents the documented double-count-then-silent-drop.

Coverage — strong. Verifier fixture plus thorough unit tests asserting real behavior (parsing edge cases, scoring, pairing guards, ragged repeats, truncation/failure_reason, HTTP 200/422 routes). Third-party C/C++ correctly kept out of the repo via gitignore + committed synthetic smoke data.

Nothing to block or gate on.

[cwing-nvidia]

Thanks @bogdankostic for this contribution! I had a few questions about the intended behavior:

1. Which PrimeVul release should this represent? The PR currently uses the v0.1 test set with 435 pairs and 870 rows, while the paper reports 564 test pairs and 1,128 rows. The upstream README notes that v0.1 excludes samples without retrievable metadata. Could you confirm whether the v0.1 subset is intentional? If so, we should label it clearly and avoid presenting it as the paper’s canonical split.

2. Which prompting protocol are you targeting? The current prompt is zero-shot, while the paper’s reported GPT results used two-shot examples or chain-of-thought. Could you confirm whether zero-shot is intentional? If we want comparability with the published results, we should include the two demonstrations.

3. What should the canonical decoding settings be? The upstream experiment used greedy decoding with temperature 0, a 1,024-token output limit, and seed 1337. The current config leaves these to provider defaults. It may be preferable to pin them for reproducibility.

4. How should incomplete pair coverage affect the headline metric? The current aggregation skips a pair when one member is missing from a repeat. This can make paired accuracy look better after failed or partial rollouts. Would you prefer canonical runs to require complete coverage, or should we report explicit scored and missing pair counts?

5. How should unfinished reasoning blocks be handled? A response such as Maybe YES may produce a YES because only closed reasoning blocks are removed. Should an unfinished reasoning block instead be treated as unanswered?

Agent harness clarification
We also recently added allowed_agents to resources server configs. When provided, Gym prevents the resources server from being paired with an unlisted harness through --agent-type. When omitted, as in this PR, every agent type is considered allowed.

The README names simple_agent, but final_assistant_text() appears designed to support multi-turn harnesses too. Could you clarify the intention? If only simple_agent has been validated, we can set:

# resources_servers/primevul/configs/primevul.yaml

primevul:
  resources_servers:
    primevul:
      entrypoint: app.py
      allowed_agents: [simple_agent]
      domain: coding

If PrimeVul is intentionally harness-agnostic, leaving it unset is appropriate, but it would be helpful to document that and ideally test at least one alternate harness.

A few additional code-level suggestions:

1. The source dataset pairs look valid, but aggregation currently treats any group with len(members) >= 2 as complete. Could we require exactly one vulnerable and one benign member? This would prevent malformed or duplicated rollout records from being scored as valid pairs.

2. The tolerant verdict parser can misread a response that quotes both options and then gives a final answer. For example, Option (1)... Option (2)... Final answer: YES is parsed as NO because numbered options take precedence. Could we select the final explicit answer by position instead?

3. Unanswered rows are currently converted into the opposite of their gold label. Penalizing them for reward and P-C makes sense, but this causes precision, recall, and confusion counts to represent predictions the model never made. Could we exclude unanswered rows from those classification metrics and report their coverage separately?

4. On a fresh checkout, primevul_benchmark.jsonl does not exist until preparation. Could we move gym eval prepare --benchmark primevul before the validation and publishing commands in the README?