Skip to content

Commit e583f85

Browse files
refactor: recover per-token ids and logprobs from content[] logprobs shape
This client always requests boolean logprobs=True, so every response uses the new content[] logprobs shape. Read the exact per-token ids and logprobs together from content[] (token_id + sampling_logprob) so they are aligned by construction. Bug 1 (alignment / KLD corruption) — the real fix: previously ids were read only from the top-level choices[].token_ids (populated by return_token_ids, which this client never requested) and, when absent, silently re-derived via tokenizer.encode(decode(text)). Retokenization drops the trailing end-of-turn/EOS token, making completion_ids one shorter than the logprobs and misaligning every per-token logprob (inference_kld ~60 vs ~0.028). Since content[] already carries token_id per entry, reading ids there removes the drift entirely — no return_token_ids, no re-encode fallback. Bug 2 (precision) — more of a feature: prefer content[].sampling_logprob (the exact value the sampler drew with) over content[].logprob (rounded). - Drop legacy token_logprobs / top-level token_ids / raw_output id sources. - Fail loud when content[] is absent or a content[] entry lacks token_id, instead of silently returning corrupted data. Co-authored-by: Cursor <cursoragent@cursor.com>
1 parent 64356d0 commit e583f85

2 files changed

Lines changed: 82 additions & 88 deletions

File tree

eval_protocol/integrations/fireworks_v1_completions_client.py

Lines changed: 43 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,19 @@ def _normalize_token_id_sequence(values: Any) -> List[int]:
8383
return [int(x) for x in list(values)]
8484

8585

86+
def _extract_entry_logprob(entry: Dict[str, Any]) -> float:
87+
"""Return the per-token logprob from a ``content[]`` logprobs entry.
88+
89+
Prefer ``sampling_logprob`` (the exact, full-precision value the sampler
90+
actually drew with) over ``logprob`` (a rounded/verification value). The
91+
sampler value is what RL training needs for accurate inference KLD.
92+
"""
93+
value = entry.get("sampling_logprob")
94+
if value is None:
95+
value = entry.get("logprob", 0.0)
96+
return float(value) if value is not None else 0.0
97+
98+
8699
def _coerce_message_content_to_text(content: Any) -> str:
87100
if content is None:
88101
return ""
@@ -358,12 +371,6 @@ async def create_completion_from_prompt_ids(
358371
}
359372
if not self.logprobs:
360373
request_payload.pop("logprobs", None)
361-
# Always request the exact generated token ids so downstream RL training
362-
# can align per-token logprobs to token ids position-by-position.
363-
# Re-encoding decoded text drops the trailing end-of-turn/EOS token and
364-
# misaligns logprobs, corrupting inference KLD. Keep overridable via
365-
# request_params, but default the flag on.
366-
request_payload.setdefault("return_token_ids", True)
367374

368375
max_retries = 40
369376
base_delay = 10.0
@@ -399,52 +406,44 @@ async def create_completion_from_prompt_ids(
399406
finish_reason = str(choice.get("finish_reason") or "unknown")
400407

401408
raw_output = choice.get("raw_output") if isinstance(choice.get("raw_output"), dict) else {}
402-
completion_token_ids = _normalize_token_id_sequence(
403-
choice.get("token_ids") or raw_output.get("completion_token_ids") or []
404-
)
405-
if not completion_token_ids:
406-
raise RuntimeError(
407-
"Fireworks /v1/completions returned no exact completion token IDs "
408-
"(choices[].token_ids and raw_output.completion_token_ids both empty) "
409-
"even though return_token_ids=True was requested. "
410-
"Refusing to re-encode decoded text: retokenization drops the "
411-
"end-of-turn token and misaligns per-token logprobs, corrupting "
412-
f"inference KLD. choice keys={list(choice.keys())}"
413-
)
414409
choice_prompt_token_ids = _normalize_token_id_sequence(
415410
choice.get("prompt_token_ids") or raw_output.get("prompt_token_ids") or normalized_prompt_token_ids
416411
)
417412

418-
completion_text = self.decode_token_ids(token_ids=completion_token_ids)
419-
if not completion_text:
420-
completion_text = str(choice.get("text") or "")
421-
422-
# -- Extract logprobs -----------------------------------------------
423-
completion_logprobs: List[float] = []
413+
# -- Extract per-token ids and logprobs together --------------------
414+
# Both come from the same ``content[]`` array entry-by-entry, so they are
415+
# inherently the same length and aligned. Reading ids from a different
416+
# source (top-level token_ids) and re-encoding decoded text when it is
417+
# absent drops the trailing end-of-turn token and misaligns per-token
418+
# logprobs, corrupting inference KLD — so we never do that.
424419
choice_logprobs = choice.get("logprobs")
425-
if isinstance(choice_logprobs, dict):
426-
token_logprobs = choice_logprobs.get("token_logprobs") or []
427-
if token_logprobs:
428-
completion_logprobs = [float(lp) if lp is not None else 0.0 for lp in token_logprobs]
429-
else:
430-
content_logprobs = choice_logprobs.get("content") or []
431-
completion_logprobs = [
432-
float(entry.get("logprob", 0.0)) if isinstance(entry, dict) else 0.0
433-
for entry in content_logprobs
434-
]
435-
elif isinstance(choice_logprobs, list):
436-
completion_logprobs = [float(lp) if lp is not None else 0.0 for lp in choice_logprobs]
437-
438-
# Catch any residual token-id / logprob drift at the boundary rather than
439-
# as a downstream KLD anomaly.
440-
if completion_logprobs and len(completion_token_ids) != len(completion_logprobs):
420+
content_entries = choice_logprobs.get("content") if isinstance(choice_logprobs, dict) else None
421+
if not content_entries:
441422
raise RuntimeError(
442-
"Fireworks /v1/completions returned mismatched completion token "
443-
f"ids ({len(completion_token_ids)}) and logprobs "
444-
f"({len(completion_logprobs)}). Per-token logprobs cannot be "
445-
"aligned to token ids; refusing to return corrupted data."
423+
"Fireworks /v1/completions returned no content[] logprobs entries. "
424+
"This client requires the boolean logprobs=True (content) shape to "
425+
"recover exact per-token ids and sampling logprobs. Refusing to "
426+
"re-encode decoded text: retokenization drops the end-of-turn token "
427+
"and misaligns per-token logprobs, corrupting inference KLD. "
428+
f"choice keys={list(choice.keys())}"
446429
)
447430

431+
completion_token_ids: List[int] = []
432+
completion_logprobs: List[float] = []
433+
for index, entry in enumerate(content_entries):
434+
if not isinstance(entry, dict) or entry.get("token_id") is None:
435+
raise RuntimeError(
436+
"Fireworks /v1/completions content[] entry is missing token_id "
437+
f"at index {index}; cannot align per-token logprobs to token ids "
438+
"without re-encoding. Refusing to return corrupted data."
439+
)
440+
completion_token_ids.append(int(entry["token_id"]))
441+
completion_logprobs.append(_extract_entry_logprob(entry))
442+
443+
completion_text = self.decode_token_ids(token_ids=completion_token_ids)
444+
if not completion_text:
445+
completion_text = str(choice.get("text") or "")
446+
448447
# -- Build message via parser or raw --------------------------------
449448
if self.tool_call_parser is not None:
450449
parsed_output = self.tool_call_parser(completion_text, completion_token_ids, active_tools)

tests/test_fireworks_v1_completions_client.py

Lines changed: 39 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -152,95 +152,86 @@ async def close(self):
152152
return captured
153153

154154

155-
def test_request_payload_sets_return_token_ids(monkeypatch):
155+
def test_reads_ids_and_sampling_logprobs_from_content(monkeypatch):
156156
client = FireworksV1CompletionsClient(
157157
model_id="test-model",
158158
tokenizer_name_or_path="Qwen/Qwen3-0.6B",
159159
)
160-
monkeypatch.setattr(client, "decode_token_ids", lambda token_ids: "hello")
160+
monkeypatch.setattr(client, "decode_token_ids", lambda token_ids: "text")
161+
162+
def _fail_tokenizer():
163+
raise AssertionError("tokenizer must not be used to re-encode completion text")
164+
165+
monkeypatch.setattr(client, "_get_tokenizer", lambda: _fail_tokenizer())
161166
captured = _install_fake_completion(
162167
client,
163168
monkeypatch,
164169
{
165170
"choices": [
166171
{
167-
"token_ids": [5, 6, 7],
168172
"finish_reason": "stop",
169-
"logprobs": {"token_logprobs": [-0.1, -0.2, -0.3]},
173+
"logprobs": {
174+
"content": [
175+
{"token_id": 271, "sampling_logprob": -0.05483185, "logprob": -0.0548313},
176+
{"token_id": 248068, "sampling_logprob": -0.0014, "logprob": -0.0014},
177+
{"token_id": 26108, "logprob": -1.0},
178+
]
179+
},
170180
}
171181
],
172182
},
173183
)
174184
result = asyncio.run(client.create_completion_from_prompt_ids(prompt_token_ids=[1, 2]))
175-
assert captured["return_token_ids"] is True
176-
assert "raw_output" not in captured
177-
assert result["completion_ids"] == [5, 6, 7]
185+
assert "return_token_ids" not in captured
186+
assert result["completion_ids"] == [271, 248068, 26108]
187+
assert result["completion_logprobs"] == [-0.05483185, -0.0014, -1.0]
178188
assert len(result["completion_ids"]) == len(result["completion_logprobs"])
179189
asyncio.run(client.close())
180190

181191

182-
def test_request_params_can_override_flags(monkeypatch):
192+
def test_raises_when_no_content_logprobs(monkeypatch):
183193
client = FireworksV1CompletionsClient(
184194
model_id="test-model",
185195
tokenizer_name_or_path="Qwen/Qwen3-0.6B",
186-
request_params={"return_token_ids": False},
187196
)
188-
monkeypatch.setattr(client, "decode_token_ids", lambda token_ids: "hi")
189-
captured = _install_fake_completion(
197+
_install_fake_completion(
190198
client,
191199
monkeypatch,
192-
{"choices": [{"token_ids": [9], "finish_reason": "stop"}]},
200+
{"choices": [{"text": "hello world", "finish_reason": "stop"}]},
193201
)
194-
asyncio.run(client.create_completion_from_prompt_ids(prompt_token_ids=[1]))
195-
assert captured["return_token_ids"] is False
202+
with pytest.raises(RuntimeError, match="no content\\[\\] logprobs entries"):
203+
asyncio.run(client.create_completion_from_prompt_ids(prompt_token_ids=[1, 2]))
196204
asyncio.run(client.close())
197205

198206

199-
def test_uses_exact_token_ids_without_reencode(monkeypatch):
207+
def test_ignores_legacy_token_logprobs_shape(monkeypatch):
208+
"""The legacy token_logprobs shape has no content[]; the client must not use it."""
200209
client = FireworksV1CompletionsClient(
201210
model_id="test-model",
202211
tokenizer_name_or_path="Qwen/Qwen3-0.6B",
203212
)
204-
monkeypatch.setattr(client, "decode_token_ids", lambda token_ids: "text")
205-
206-
def _fail_encode():
207-
raise AssertionError("tokenizer must not be used to re-encode completion text")
208-
209-
monkeypatch.setattr(client, "_get_tokenizer", lambda: _fail_encode())
210213
_install_fake_completion(
211214
client,
212215
monkeypatch,
213216
{
214217
"choices": [
215218
{
216-
"token_ids": [10, 20, 30, 40],
217219
"finish_reason": "stop",
218-
"logprobs": {"token_logprobs": [-0.1, -0.2, -0.3, -0.4]},
220+
"token_ids": [1, 2, 3],
221+
"logprobs": {
222+
"token_ids": [1, 2, 3],
223+
"token_logprobs": [-0.1, -0.2, -0.3],
224+
},
219225
}
220226
],
221227
},
222228
)
223-
result = asyncio.run(client.create_completion_from_prompt_ids(prompt_token_ids=[1]))
224-
assert result["completion_ids"] == [10, 20, 30, 40]
225-
asyncio.run(client.close())
226-
227-
228-
def test_raises_when_no_exact_token_ids(monkeypatch):
229-
client = FireworksV1CompletionsClient(
230-
model_id="test-model",
231-
tokenizer_name_or_path="Qwen/Qwen3-0.6B",
232-
)
233-
_install_fake_completion(
234-
client,
235-
monkeypatch,
236-
{"choices": [{"text": "hello world", "finish_reason": "stop"}]},
237-
)
238-
with pytest.raises(RuntimeError, match="no exact completion token IDs"):
239-
asyncio.run(client.create_completion_from_prompt_ids(prompt_token_ids=[1, 2]))
229+
with pytest.raises(RuntimeError, match="no content\\[\\] logprobs entries"):
230+
asyncio.run(client.create_completion_from_prompt_ids(prompt_token_ids=[1]))
240231
asyncio.run(client.close())
241232

242233

243-
def test_raises_on_id_logprob_length_mismatch(monkeypatch):
234+
def test_raises_when_content_entry_missing_token_id(monkeypatch):
244235
client = FireworksV1CompletionsClient(
245236
model_id="test-model",
246237
tokenizer_name_or_path="Qwen/Qwen3-0.6B",
@@ -252,14 +243,18 @@ def test_raises_on_id_logprob_length_mismatch(monkeypatch):
252243
{
253244
"choices": [
254245
{
255-
"token_ids": [1, 2, 3],
256246
"finish_reason": "stop",
257-
"logprobs": {"token_logprobs": [-0.1, -0.2, -0.3, -0.4]},
247+
"logprobs": {
248+
"content": [
249+
{"token_id": 1, "sampling_logprob": -0.1},
250+
{"sampling_logprob": -0.2},
251+
]
252+
},
258253
}
259254
],
260255
},
261256
)
262-
with pytest.raises(RuntimeError, match="mismatched completion token"):
257+
with pytest.raises(RuntimeError, match="missing token_id"):
263258
asyncio.run(client.create_completion_from_prompt_ids(prompt_token_ids=[1]))
264259
asyncio.run(client.close())
265260

0 commit comments

Comments
 (0)