Strict prompt construction in load_test.py (minimal)#120
Strict prompt construction in load_test.py (minimal)#120ishaan-shivhare wants to merge 1 commit into
Conversation
Use build_pair_ids from prefill_load_test for exact token-id prompts with a deterministic shared prefix. Keep the legacy text path for rerank so documents are still paragraph-split strings. Preserve --prompt as an encoded suffix on strict prompts. Wire token ids through format_payload and /v1/completions routing. Co-authored-by: Ishaan Shivhare <ishaan-shivhare@users.noreply.github.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 383af18. Configure here.
|
|
||
| url = self.provider_formatter.get_url() | ||
| if isinstance(prompt, list) and prompt and isinstance(prompt[0], int): | ||
| url = "/v1/completions" |
There was a problem hiding this comment.
Chat parser mismatches completions stream
High Severity
Token-id prompts force /v1/completions, but OpenAIProvider.parse_output_json still branches on --chat (default true) and reads delta/message content fields. Completions streaming exposes choices[].text, so decoded output stays empty and requests can be marked failed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 383af18. Configure here.
|
|
||
| url = self.provider_formatter.get_url() | ||
| if isinstance(prompt, list) and prompt and isinstance(prompt[0], int): | ||
| url = "/v1/completions" |
There was a problem hiding this comment.
Embeddings URL overridden incorrectly
Medium Severity
The token-id URL override sets /v1/completions for any int list prompt, including when --embeddings built an /v1/embeddings body. That sends embedding JSON to the completions route and breaks embeddings runs on strict datasets.
Reviewed by Cursor Bugbot for commit 383af18. Configure here.
| if isinstance(sample_prompt, list): | ||
| self.prompt_tokenizer_tokens = len(sample_prompt) | ||
| else: | ||
| self.prompt_tokenizer_tokens = len(tokenizer.encode(sample_prompt)) |
There was a problem hiding this comment.
Image placeholders assume string prompts
Medium Severity
With --prompt-images, _get_input still runs insert_image_placeholders on strict token-id prompts. That helper slices and concatenates strings, so list prompts raise TypeError or corrupt placeholders instead of valid multimodal input.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 383af18. Configure here.
2 participants
Summary
Makes limericks/code prompt construction exact for completions workloads (PER-75), with a minimal diff scoped to prompt building and request wiring only.
What changed
TranslationDataset: strict mode (default) usesbuild_pair_ids/build_ids_to_lengthfromprefill_load_test.pyto produce exact token-id prompts with a deterministic shared prefix--promptpreserved: custom instruction is encoded and appended before the chat suffix, same as the old text suffixstrict=Falsewhen--rerankis set, so prompts stay text strings andformat_payloadstill paragraph-splits them into documents (fixes Bugbot rerank regression)/v1/completionsviaformat_payload+ inline URL override in_do_generate_text(noget_url(prompt)change that broke other providers)--forced-generation-from-datasetis usedWhat did NOT change
prefill_load_test.pyTests
7 tests: exact lengths, shared prefix, custom
--promptsuffix, rerank text mode, token-id payload wiring.vs prior PR #119
Previous PR rewired
DatasetHolder(dropped--prompt, always token ids including rerank). This version keeps legacy text construction for rerank and only strictifies completions prompt building.Slack Thread