fix: make validate_repeat_prompt strip/case-insensitive#1774
fix: make validate_repeat_prompt strip/case-insensitive#1774Chessing234 wants to merge 2 commits into
Conversation
Match IFEvalG so leading whitespace and case no longer fail the check. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.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 updates the validate_repeat_prompt function to strip whitespace and perform case-insensitive comparisons, aligning it with IFEvalG, and adds corresponding unit tests. The review feedback suggests removing a redundant bool() cast, adding a test case to verify handling of whitespace in the original prompt, and replacing the placeholder PR number in the changelog.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| ### Removed | ||
|
|
||
| ### Fixed | ||
| - Make `validate_repeat_prompt` strip/case-insensitive (https://github.com/allenai/open-instruct/pull/1774). |
There was a problem hiding this comment.
This changelog entry contains a placeholder XXXX for the pull request number. Please update it with the actual PR number before merging to ensure the changelog is accurate.
| def validate_repeat_prompt(text: str, original_prompt: str) -> bool: | ||
| return bool(text.startswith(original_prompt)) | ||
| """Match IFEvalG: strip and compare case-insensitively.""" | ||
| return bool(text.strip().lower().startswith(original_prompt.strip().lower())) |
There was a problem hiding this comment.
The bool() cast is redundant here since str.startswith() already returns a boolean. Removing it will make the code slightly cleaner.
| return bool(text.strip().lower().startswith(original_prompt.strip().lower())) | |
| return text.strip().lower().startswith(original_prompt.strip().lower()) |
| [ | ||
| ("exact", "Hello world answer", "Hello world", True), | ||
| ("leading_space", " Hello world answer", "Hello world", True), | ||
| ("case", "HELLO world answer", "Hello world", True), | ||
| ("missing", "Something else", "Hello world", False), | ||
| ] |
There was a problem hiding this comment.
The new tests cover changes to text well. To make the test suite more comprehensive, consider adding a test case to verify that original_prompt with leading or trailing whitespace is handled correctly by strip(). For example: ('prompt_with_space', 'Hello world answer', ' Hello world ', True)
1 participant
Summary
startswith, matching IFEvalG.Test plan