fix: populate prefix_cache_info in OpenAI/session rollout path#960
fix: populate prefix_cache_info in OpenAI/session rollout path#960
Conversation
The /generate path already collects prefix cache stats via update_from_meta_info, but the OpenAI endpoint path skipped it, causing rollout/prefix_cache_hit_rate to always report 0 for session-based rollouts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements prefix cache tracking by extracting metadata from OpenAI responses and populating the sample's cache information. It includes new tests for single-turn and multi-turn scenarios, as well as handling for missing fields. A review comment suggests refactoring the metadata extraction for consistency and safety.
| case "abort": | ||
| sample.status = Sample.Status.ABORTED | ||
|
|
||
| sample.prefix_cache_info.add(choice.get("meta_info", {})) |
There was a problem hiding this comment.
While this correctly populates the prefix cache information, it is inconsistent with the rest of the function which accesses meta_info directly (e.g., lines 159-160). If meta_info is missing, the function will have already raised a KeyError before reaching this line. For better consistency and safety, consider extracting meta_info once at the beginning of the function and using it throughout.
1 participant
The /generate path already collects prefix cache stats via update_from_meta_info, but the OpenAI endpoint path skipped it, causing rollout/prefix_cache_hit_rate to always report 0 for session-based rollouts.