Skip to content

perf(client): read prime-rl's packed completion_logprobs - #174

Open
faresobeid wants to merge 3 commits into
mainfrom
perf/packed-completion-logprobs
Open

faresobeid wants to merge 3 commits into
mainfrom
perf/packed-completion-logprobs

Conversation

@faresobeid

@faresobeid faresobeid commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Lets generate() read prime-rl's packed per-token logprobs.

prime-rl's /inference/v1/generate server now returns the sampled-token logprobs as one base64 float32 array, choice.completion_logprobs = {data, shape, dtype}, instead of choice.logprobs.content (one JSON object per token). The per-token objects were a large part of the vLLM API server's GC load under RL traffic (see the prime-rl PR).

  • _parse_completion_logprobs reads completion_logprobs when present and applies the same checks as the per-token path: count matches completion_ids, values finite, no -9999.0 sentinel. Returns the same list[float].
  • Without the field (stock vLLM), nothing changes.
  • sampling_mask is passed through as before. On prime-rl's server it is now a packed {ids, counts} dict; verifiers decodes it.

Companions: prime-rl PrimeIntellect-ai/prime-rl#3900 / verifiers PrimeIntellect-ai/verifiers#2778. Land renderers -> verifiers -> prime-rl; the prime-rl submodule pins must move to the merged commits.

Validation

  • tests/test_client.py: 32 passed (one new test for the packed path, incl. the sentinel check).
  • ruff check / ruff format --check (0.15.12) on the changed files: clean.

Note

Medium Risk
Changes how sampled-token logprobs are parsed from engine responses; bugs could corrupt RL training signals, but behavior is backward compatible and guarded by existing validation plus new tests.

Overview
Adds support for prime-rl’s packed per-token completion logprobs on /inference/v1/generate responses, so generate() can avoid parsing the heavy OpenAI-style choice.logprobs.content list under RL traffic.

When choice.completion_logprobs is present ({data, shape, dtype} with base64 float32), _parse_completion_logprobs decodes it via a new _parse_packed_completion_logprobs helper and still returns the same list[float] as before. Validation mirrors the legacy path: length must match completion_ids, values must be finite, and vLLM’s -9999 missing-logprob sentinel is rejected. Stock vLLM responses without the field keep using logprobs.content unchanged.

A comment notes that prime-rl may also return sampling_mask as packed CSR {ids, counts} (still passed through; decoding stays downstream). Tests cover successful packed decoding plus sentinel and non-finite failures.

Reviewed by Cursor Bugbot for commit e134d56. Bugbot is set up for automated code reviews on this repo. Configure here.

Note

Add packed completion_logprobs decoding to client renderer

  • Adds _parse_packed_completion_logprobs in renderers/client.py, which decodes base64 packed logprob payloads into a NumPy array using the supplied dtype and returns per-token floats
  • generate now prefers choice.completion_logprobs when present, falling back to the existing choice.logprobs.content parsing
  • Length mismatches, non-finite values, and values at or below the -9999 missing-logprob sentinel raise MalformedGenerateResponseError
  • Risk: importing renderers.client now requires NumPy as a dependency

Macroscope summarized e134d56.

@faresobeid
faresobeid marked this pull request as ready for review October 7, 2026 14:31

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 8a9619d. Configure here.

Comment thread renderers/client.py
@macroscopeapp

macroscopeapp Bot commented Oct 7, 2026 •

Copy link
Copy Markdown

Approvability

Verdict: Approved at e134d56

Macroscope's review found this PR approvable — This is a localized, backward-compatible performance path for decoding packed completion logprobs; existing response handling remains unchanged and NumPy was already a required dependency. The new behavior is covered by focused tests for valid and invalid payloads.

Notes:

  • Macroscope's correctness review did not run, so approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

This branch has not been deployed

No deployments
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.

1 participant