From 1d62c1d6dc4f6608938d22424c46edef6ce50671 Mon Sep 17 00:00:00 2001 From: Jasper Lu Date: Thu, 16 Jul 2026 20:02:05 -0400 Subject: [PATCH] Fix six instruction verifiers that mis-score correct responses Six check_following verifiers assign the wrong verdict; this fixes them and adds regression tests to instructions_test.py. Full suite passes (65 tests); no new dependencies. False-fail (a correct response was rejected): - repeat:repeat_span (RepeatSpanChecker): prompt says "word indices, split by whitespace" but the checker sliced character indices, so the target was a mid-word substring. Now slices whitespace words; description/defaults use word indices. - ratio:overlap (NGramOverlapChecker): nltk.ngrams(value, 3) on the raw string produced character trigrams, not word trigrams. Now tokenizes into word trigrams. - count:keywords_multiple (KeywordsMultipleChecker): value.lower().count(kw) counted substrings ("door" inside "doorway"/"indoors", any -ed/-s/-ing form). Now counts whole words with the same \b-bounded, case-insensitive match used by PersonNameCountChecker and IncludeKeywordChecker. - custom:sentence_alphabet (SentenceAlphabetChecker): only lstrip() whitespace, so a first word wrapped in a quote made the quote the "first letter". Now strips leading punctuation. - words:words_position (WordsPositionChecker): nltk tokens included leading and trailing punctuation, shifting the 2nd / 2nd-to-last word positions. Now uses the shared _word_tokens_without_punctuation helper. False-pass (a wrong response was accepted): - ratio:sentence_words (CharacterCountUniqueWordsChecker): never checked "all different words". Now enforces word uniqueness across the three sentences. --- instructions.py | 63 ++++++++++++++++++++++++----------------- instructions_test.py | 67 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 26 deletions(-) diff --git a/instructions.py b/instructions.py index f694dfd..7e20c29 100644 --- a/instructions.py +++ b/instructions.py @@ -47,6 +47,11 @@ def _word_tokens_without_punctuation(text): if any(ch.isalnum() for ch in token) ] + +def _split_words(text): + """Lowercase, drop punctuation, and split into whitespace-delimited words.""" + return text.lower().translate(str.maketrans('', '', string.punctuation)).split() + logger = logging.getLogger(__name__) _InstructionArgsDtype = Optional[Dict[str, Union[int, str, Sequence[str]]]] @@ -437,8 +442,8 @@ def get_instruction_args_keys(self): def check_following(self, value): """Checks if the response maintains a trigram overlap with the reference text within 2% of {percent}.""" n = 3 - ngrams = set(nltk.ngrams(value, n)) - ref_ngrams = set(nltk.ngrams(self._reference_text, n)) + ngrams = set(nltk.ngrams(nltk.word_tokenize(value), n)) + ref_ngrams = set(nltk.ngrams(nltk.word_tokenize(self._reference_text), n)) if not ngrams: return False overlap = len(ngrams.intersection(ref_ngrams)) / len(ngrams) @@ -932,6 +937,11 @@ def check_following(self, value): for sentence in sentences: if len(sentence.strip()) != char_count: return False + # Check "all different words", using the same word tokenization as + # LimitedWordRepeatChecker (words:repeats). + words = _split_words(" ".join(sentences)) + if len(words) != len(set(words)): + return False return True @@ -1047,7 +1057,7 @@ def get_instruction_args_keys(self): def check_following(self, value): """Checks if the response repeats any word more than {small_n} times.""" - words = value.lower().translate(str.maketrans('', '', string.punctuation)).split() + words = _split_words(value) word_count = Counter(words) for word, count in word_count.items(): if count > self._max_repeats: @@ -1304,7 +1314,7 @@ def get_instruction_args_keys(self): def check_following(self, value): """Checks if no two consecutive words in the response share the same first letter.""" - words = value.lower().translate(str.maketrans('', '', string.punctuation)).split() + words = _split_words(value) while '' in words: words.remove('') for i in range(len(words) - 1): @@ -1701,10 +1711,11 @@ def check_following(self, value): if len(sentences) != 26: return False for i, sentence in enumerate(sentences): - words = sentence.lstrip().split() - if not words or not words[0]: + # Strip leading whitespace and punctuation so that "A lazy fox..."" counts as a sentence starting with 'A' + stripped = sentence.lstrip(string.punctuation + string.whitespace) + if not stripped: return False - if words[0].lower()[0] != chr(97 + i): + if stripped[0].lower() != chr(97 + i): return False return True @@ -1949,7 +1960,10 @@ def get_instruction_args_keys(self): def check_following(self, value): for keyword, count in zip([self._keyword1, self._keyword2, self._keyword3, self._keyword4, self._keyword5], [1, 2, 3, 5, 7]): - if value.lower().count(keyword.lower()) != count: + pattern = r'\b{}\b'.format(re.escape(keyword)) + # See if the whole word appears the expected number of times, case insensitive. + # Use regex instead of str.count to avoid counting substrings (e.g., "cat" in "concatenate"). + if len(re.findall(pattern, value, flags=re.IGNORECASE)) != count: return False return True @@ -2058,18 +2072,10 @@ def check_following(self, value): True if the second word and the second to last word are the same; otherwise, False. """ - words = instructions_util.nltk.word_tokenize(value) + words = _word_tokens_without_punctuation(value) if len(words) < 2: return False - if words[-1] in string.punctuation: - if len(words) < 3: - return False - if words[1].lower() == words[-3].lower() == self._keyword.lower(): - return True - return False - elif words[1].lower() == words[-2].lower() == self._keyword.lower(): - return True - return False + return words[1].lower() == words[-2].lower() == self._keyword.lower() class RepeatChangeChecker(Instruction): @@ -2151,14 +2157,14 @@ def check_following(self, value): class RepeatSpanChecker(Instruction): - "Copy the span of words that lies between (and including) index {n_start} and {n_end}, the indices are character indices!" + "Copy the span of words that lies between (and including) index {n_start} and {n_end}, the indices are word indices, split by whitespace!" def build_description(self, prompt_to_repeat=None, n_start=None, n_end=None): """Build the instruction description. Args: - n_start: An integer representing the inclusive start character index of the span. - n_end: An integer representing the inclusive end character index of the span. + n_start: An integer representing the inclusive start word index of the span. + n_end: An integer representing the inclusive end word index of the span. Returns: A string representing the instruction description. @@ -2167,16 +2173,20 @@ def build_description(self, prompt_to_repeat=None, n_start=None, n_end=None): raise ValueError("prompt_to_repeat must be set.") else: self._prompt_to_repeat = prompt_to_repeat + # Indices are word indices (whitespace-split), matching the instruction text + # and the check below. Previously these defaulted to character offsets, which + # contradicted the "span of words" wording and produced mid-word targets. + num_words = len(self._prompt_to_repeat.split()) if n_start is None: - self._n_start = random.randint(0, len(self._prompt_to_repeat) - 2) + self._n_start = random.randint(0, num_words - 2) else: self._n_start = n_start if n_end is None: - self._n_end = random.randint(self._n_start + 1, len(self._prompt_to_repeat) - 1) + self._n_end = random.randint(self._n_start + 1, num_words - 1) else: self._n_end = n_end self._description_pattern = ( - "Copy the span of words that lies between (and including) index {n_start} and {n_end}, the indices are character indices!") + "Copy the span of words that lies between (and including) index {n_start} and {n_end}, the indices are word indices, split by whitespace!") return self._description_pattern.format(n_start=self._n_start, n_end=self._n_end, prompt_to_repeat=self._prompt_to_repeat) @@ -2189,8 +2199,9 @@ def get_instruction_args_keys(self): return ["n_start", "n_end", "prompt_to_repeat"] def check_following(self, value): - """Checks if the response contains the expected number of phrases with the correct modifications.""" - expected_span = self._prompt_to_repeat[self._n_start:self._n_end + 1] + """Checks if the response is the whitespace-split word span [n_start, n_end] (inclusive).""" + words = self._prompt_to_repeat.split() + expected_span = ' '.join(words[self._n_start:self._n_end + 1]) if value.strip().lower() == expected_span.strip().lower(): return True return False diff --git a/instructions_test.py b/instructions_test.py index 197161d..b7f0811 100644 --- a/instructions_test.py +++ b/instructions_test.py @@ -1185,5 +1185,72 @@ def test_stop_word_percentage__returns_false_when_response_empty(self): self.assertFalse(instruction.check_following(""), "expected False for empty response") self.assertFalse(instruction.check_following(" "), "expected False for whitespace-only response") + def test_repeat_span__uses_word_indices(self): + """Test that RepeatSpanChecker slices on whitespace word indices, not characters.""" + instruction_id = 'repeat:repeat_span' + instruction = instructions.RepeatSpanChecker(instruction_id) + instruction.build_description( + prompt_to_repeat="The walls are solid but the stones are cracked and cold.", + n_start=0, n_end=7) + self.assertTrue(instruction.check_following("The walls are solid but the stones are")) + self.assertFalse(instruction.check_following("The walls are solid")) + self.assertFalse(instruction.check_following("The wall")) + + def test_ngram_overlap__measures_word_trigrams(self): + """Test that NGramOverlapChecker measures word trigram overlap.""" + instruction_id = 'ratio:overlap' + instruction = instructions.NGramOverlapChecker(instruction_id) + reference_text = "the quick brown fox jumps over the lazy dog again and again today" + instruction.build_description(reference_text=reference_text, percentage=100) + self.assertTrue(instruction.check_following(reference_text)) + self.assertFalse(instruction.check_following("completely unrelated wording appears in this line")) + + TEST_KEYWORDS_MULTIPLE_MESSAGE_1 = ( + "The doorway led indoors past the doorstep to one door. " + + "bread " * 2 + "blue " * 3 + "lamp " * 5 + "river " * 7) + TEST_KEYWORDS_MULTIPLE_MESSAGE_2 = ( + "door door " + "bread " * 2 + "blue " * 3 + "lamp " * 5 + "river " * 7) + def test_keywords_multiple__counts_whole_words(self): + """Test that KeywordsMultipleChecker counts whole words rather than substrings.""" + instruction_id = 'count:keywords_multiple' + instruction = instructions.KeywordsMultipleChecker(instruction_id) + instruction.build_description(keyword1="door", keyword2="bread", keyword3="blue", + keyword4="lamp", keyword5="river") + with self.subTest('test with TEST_KEYWORDS_MULTIPLE_MESSAGE_1'): + self.assertTrue(instruction.check_following(self.TEST_KEYWORDS_MULTIPLE_MESSAGE_1)) + with self.subTest('test with TEST_KEYWORDS_MULTIPLE_MESSAGE_2'): + self.assertFalse(instruction.check_following(self.TEST_KEYWORDS_MULTIPLE_MESSAGE_2)) + + def test_sentence_alphabet__ignores_leading_punctuation(self): + """Test that SentenceAlphabetChecker ignores a leading quote or markdown on the first word.""" + instruction_id = 'custom:sentence_alphabet' + instruction = instructions.SentenceAlphabetChecker(instruction_id) + instruction.build_description() + words = ["Apple", "Bears", "Cats", "Dogs", "Eagles", "Foxes", "Goats", "Hawks", "Ibex", + "Jays", "Kites", "Lions", "Moose", "Newts", "Owls", "Pigs", "Quail", "Rats", + "Seals", "Toads", "Urial", "Voles", "Wolves", "Xerus", "Yaks", "Zebras"] + sentences = [f"{word} appear here now." for word in words] + self.assertTrue(instruction.check_following(" ".join(sentences))) + self.assertTrue(instruction.check_following('"' + sentences[0] + " " + " ".join(sentences[1:]))) + self.assertFalse(instruction.check_following(" ".join([sentences[1], sentences[0]] + sentences[2:]))) + + def test_words_position__ignores_punctuation_tokens(self): + """Test that WordsPositionChecker computes positions on words, ignoring punctuation tokens.""" + instruction_id = 'words:words_position' + instruction = instructions.WordsPositionChecker(instruction_id) + instruction.build_description(keyword="vibrant") + self.assertTrue(instruction.check_following("The vibrant sun set over a calm vibrant sea")) + self.assertTrue(instruction.check_following('"The vibrant sun set over a calm vibrant sea!"')) + self.assertFalse(instruction.check_following("The lazy sun set over a calm quiet sea")) + + def test_sentence_words__requires_unique_words(self): + """Test that CharacterCountUniqueWordsChecker enforces the all-different-words requirement.""" + instruction_id = 'ratio:sentence_words' + instruction = instructions.CharacterCountUniqueWordsChecker(instruction_id) + instruction.build_description() + self.assertFalse(instruction.check_following("The cat sat here. The cat sat here. The cat sat here.")) + self.assertTrue(instruction.check_following("This is one. Now it's 22. On to three.")) + + if __name__ == '__main__': absltest.main()