mirror of
https://github.com/QwenLM/qwen-code.git
synced 2026-07-09 17:19:02 +00:00
* fix(core): skip abbreviations in multiple_sentences filter (#6077) The follow-up suggestion generator was falsely flagging suggestions containing common abbreviations like 'vs.' or 'Dr.' as multiple sentences, because the filter regex matched any period followed by a capitalized word. Replace the inline regex with a named MULTIPLE_SENTENCES_RE that uses a negative lookbehind to skip common honorifics (Mr, Mrs, Dr, Ms, Prof, Sr, Jr, St), abbreviations (vs, etc), and Latin shorthands (e.g., i.e.). Add vitest coverage for the issue's reported case, the common abbreviations, and an explicit multi-sentence case so the filter still catches real run-on suggestions. Fixes #6077 * fix(core): replace blanket abbreviation lookbehind with per-boundary check (#6077) The negative lookbehind in MULTIPLE_SENTENCES_RE exempted every abbreviation (including etc, eg, ie) from sentence-boundary detection, so 'Review options etc. Then commit.' incorrectly passed the filter. Replace the regex with hasSentenceBoundary(), which checks each potential boundary individually: honorifics (Dr., Mr., etc.) and vs. are recognized as safe continuations, while etc. at the end of a real sentence is still correctly detected. Latin shorthands (e.g., i.e.) are handled by checking the two-part pattern before the period. * fix(core): add etc to abbreviation set, rename constant, strengthen tests (#6077) * fix(core): address review feedback (#6077) - Make e.g./i.e. abbreviation check case-insensitive to handle capitalized variants (E.g., I.e.) - Add test for non-word-char-before-punctuation branch in hasSentenceBoundary --------- Co-authored-by: Qwen Autofix Bot <autofix@qwen-code.dev> Co-authored-by: Qwen Code <qwen-code@alibaba.com>
This commit is contained in:
parent
ba7561af49
commit
62c0a0a772
2 changed files with 66 additions and 1 deletions
|
|
@ -214,6 +214,38 @@ describe('shouldFilterSuggestion', () => {
|
|||
|
||||
it('filters multiple sentences', () => {
|
||||
expect(shouldFilterSuggestion('Run the tests. Then commit.')).toBe(true);
|
||||
expect(shouldFilterSuggestion('Hello! How are you?')).toBe(true);
|
||||
expect(shouldFilterSuggestion('Do this. Then do that.')).toBe(true);
|
||||
// Abbreviation skipped, then real sentence boundary detected
|
||||
expect(shouldFilterSuggestion('Check Dr. Smith. Then commit.')).toBe(true);
|
||||
// Non-word char before punctuation — still detected as sentence boundary
|
||||
expect(shouldFilterSuggestion('Run (see docs). Then deploy')).toBe(true);
|
||||
});
|
||||
|
||||
it('does not filter abbreviations as multiple sentences', () => {
|
||||
// Issue #6077 — "vs." followed by a capitalized word should pass.
|
||||
expect(
|
||||
shouldFilterSuggestion(
|
||||
"Let's start with the Weeds vs. Wildflowers audit.",
|
||||
),
|
||||
).toBe(false);
|
||||
expect(shouldFilterSuggestion('Weeds vs. Wildflowers audit')).toBe(false);
|
||||
// Common honorifics and abbreviations
|
||||
expect(shouldFilterSuggestion('Check Dr. Smith notes')).toBe(false);
|
||||
expect(shouldFilterSuggestion('Ask Mr. Jones for help')).toBe(false);
|
||||
expect(shouldFilterSuggestion('Talk to Ms. Patel next')).toBe(false);
|
||||
expect(shouldFilterSuggestion('See Prof. Lee today')).toBe(false);
|
||||
expect(shouldFilterSuggestion('Visit St. Petersburg office')).toBe(false);
|
||||
expect(shouldFilterSuggestion('Check etc. Tasks remaining')).toBe(false);
|
||||
expect(shouldFilterSuggestion('Review options etc. Then commit')).toBe(
|
||||
false,
|
||||
);
|
||||
// Latin shorthands with an internal period
|
||||
expect(shouldFilterSuggestion('Use e.g. Docker to build')).toBe(false);
|
||||
expect(shouldFilterSuggestion('Use i.e. Docker to build')).toBe(false);
|
||||
// Capitalized variants still recognized as abbreviations
|
||||
expect(shouldFilterSuggestion('Use E.g. Docker to build')).toBe(false);
|
||||
expect(shouldFilterSuggestion('Use I.e. Docker to build')).toBe(false);
|
||||
});
|
||||
|
||||
it('filters formatting', () => {
|
||||
|
|
|
|||
|
|
@ -239,6 +239,39 @@ const ALLOWED_SINGLE_WORDS = new Set([
|
|||
'no',
|
||||
]);
|
||||
|
||||
const KNOWN_ABBREVIATIONS = new Set([
|
||||
'Mr',
|
||||
'Mrs',
|
||||
'Dr',
|
||||
'Ms',
|
||||
'Prof',
|
||||
'Sr',
|
||||
'Jr',
|
||||
'St',
|
||||
'vs',
|
||||
'etc',
|
||||
]);
|
||||
|
||||
const SENTENCE_BOUNDARY_RE = /[.!?]\s+[A-Z]/g;
|
||||
|
||||
function hasSentenceBoundary(suggestion: string): boolean {
|
||||
for (const m of suggestion.matchAll(SENTENCE_BOUNDARY_RE)) {
|
||||
const i = m.index!;
|
||||
const before = suggestion.slice(0, i);
|
||||
const wordMatch = before.match(/(\w+)$/);
|
||||
if (!wordMatch) return true;
|
||||
const word = wordMatch[1];
|
||||
if (KNOWN_ABBREVIATIONS.has(word)) continue;
|
||||
if (
|
||||
(word === 'g' && /e\.g$/i.test(before)) ||
|
||||
(word === 'e' && /i\.e$/i.test(before))
|
||||
)
|
||||
continue;
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns the filter reason if the suggestion should be suppressed, or null if it passes.
|
||||
*/
|
||||
|
|
@ -299,7 +332,7 @@ export function getFilterReason(suggestion: string): string | null {
|
|||
if (suggestion.length > 30) return 'too_many_words';
|
||||
}
|
||||
if (suggestion.length >= 100) return 'too_long';
|
||||
if (/[.!?]\s+[A-Z]/.test(suggestion)) return 'multiple_sentences';
|
||||
if (hasSentenceBoundary(suggestion)) return 'multiple_sentences';
|
||||
if (/[\n*]|\*\*/.test(suggestion)) return 'has_formatting';
|
||||
|
||||
if (
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue