feat: log toolcall and thinktag violation rate into single controller

[arnavk-nvidia]

What does this PR do ?

Logs metrics of toolcall and think tag violation rate in single controller similar to the one existing in grpo.py.
In single controller the message log gets dropped hence the metrics are calculated in payload itself.

Issues

List issues that this PR closes (syntax):

Usage

• You can potentially add a usage example below

# Add a code snippet demonstrating how to use this

Before your PR is "Ready for review"

Pre checks:

• Make sure you read and followed Contributor guidelines
• Did you write any new necessary tests?
• Did you run the unit tests and functional tests locally? Visit our Testing Guide for how to run tests
• Did you add or update any necessary documentation? Visit our Document Development Guide for how to write, build and test the docs.

Additional Information

• ...

[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.

[arnavk-nvidia]

Automated review found one bug that will fail CI.

Generated by Claude Code

[arnavk-nvidia]

/ok to test c51e9ff

[yfw]

Thanks for closing this gap — the SC path had no violation telemetry while the sync GRPO path did, and riding the documented tags sidecar with metric names matching grpo.py is the right shape for it. Nice quick turnaround on the keyword-arg fix in c51e9ff too.

2 action items, 1 follow-up. Findings re-verified against c51e9ff after the force-push.

The branch is still red on tests it doesn't touch. addopts carries -x, so a plain run stops at the first failure and hides the rest:

$ uv run --group test pytest tests/unit/single_controller/ tests/unit/experience/ -q --maxfail=0
7 failed, 702 passed

AI-1 — _train_pump_controller needs the new _step_log_dict keys

All 7 remaining failures are test_train_pump_* / test_advantage_stage_* in tests/unit/single_controller/test_single_controller_actor.py — a file this PR doesn't touch — and they share one cause:

    for tag in meta.tags or []:
        for key in VIOLATION_TAG_KEYS:
>           self._step_log_dict[key].append(int(tag.get(key, 0)))
E           KeyError: 'num_invalid_tool_calls'
nemo_rl/algorithms/single_controller.py:1967: KeyError

The new drain loop hard-indexes _step_log_dict. Production seeds every key at single_controller.py:313-319, but _train_pump_controller hand-builds it with four at line 1112, and its metas carry tags, so the loop actually runs. That file is unchanged from main, so this is a regression from the production change rather than a stale test.

Action: add the keys to the fixture, plus the import. Verified locally — 45 passed on that file, from 7 failed.

# tests/unit/single_controller/test_single_controller_actor.py
from nemo_rl.experience.payload import VIOLATION_TAG_KEYS
...
    ctrl._step_log_dict = {
        "rewards": [],
        "masked_advantages": [],
        "sequence_lengths": [],
        "seq_logprob_error_metrics": [],
        **{key: [] for key in VIOLATION_TAG_KEYS},
    }

Please fix in this PR — CI stays red otherwise. Worth fixing in the fixture rather than by moving the drain loop below the _advantage_estimator is None early-out: the other _advantage_stage tests pass only because their metas have no tags, which is a fragile reason to be green.

AI-2 — one comment on VIOLATION_TAG_KEYS

VIOLATION_TAG_KEYS is now load-bearing in four independent places: the tuple itself, the __init__ seed, the reduce_advantage_pump_metrics signature, and that test fixture. Nothing links them statically — the **self._step_log_dict splat at single_controller.py:1081 is opaque to both ruff and pyrefly. The KeyError above is one failure mode; adding a 4th key to the tuple without editing the utils.py signature is the other, and it surfaces as TypeError: ... unexpected keyword argument at the first train step.

Action: a one-line comment on the tuple ("keep in sync with reduce_advantage_pump_metrics kwargs"). Explicitly not asking for a refactor — collapsing the three kwargs into a single dict parameter moves the complexity rather than removing it, and it would break the deliberate metric-name parity with grpo.py that motivates this PR.

Follow-up — an assertion on the drained counts

Nothing currently asserts what the drain loop does: the other _advantage_stage tests build metas without tags, so it runs zero iterations, and the 7 above only enter it incidentally. Since you're editing that fixture anyway, an assertion on the drained counts would also pin the tag.get(key, 0) default for rows enqueued before this feature — the rolling-upgrade case. Fine as a follow-up if you'd rather keep this PR tight.

Context — no action

ruff check and pyrefly check are clean on all changed files. We looked at these and are explicitly not asking for them: popping the non-tensor violation_counts key out of the BatchedDataDict (it's inert — pack_payload filters on isinstance(v, torch.Tensor) and the batch is dropped right after); using stamp_tags instead of pack_payload (wrong lifecycle — stamp_tags is for scalars derived later in the train loop); and unifying _violation_counts with grpo's counting loop (that loop also writes penalty advantages and threads token_offset, so merging them would be over-abstraction).

Generated by Claude Code

[arnavk-nvidia]

/ok to test 8c2ba3d

[arnavk-nvidia]

/ok to test af2c889

[yfw]

/ok to test 59fb697