Skip to content

Commit e3f63ff

Browse files
authored
Merge pull request #614 from efegokdemir/codex/issue-609-tr1-discourse-boundary
fix: preserve bounded trigger phrases
2 parents c35d9d3 + 91a925a commit e3f63ff

2 files changed

Lines changed: 12 additions & 39 deletions

File tree

‎src/skillspector/nodes/analyzers/static_patterns_supply_chain.py‎

Lines changed: 5 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -625,19 +625,6 @@ def _is_typosquat(pkg_name: str, popular: set[str], max_distance: int = 2) -> st
625625
re.IGNORECASE,
626626
)
627627

628-
# Trailing discourse words that modify the utterance rather than name trigger
629-
# content. When the captured phrase is a broad single word followed only by
630-
# these deictics ("says hello there"), the skill names the broad word and the
631-
# rest is trailing prose, so TR1 still fires; a content word after the broad
632-
# word ("code review", "the zone") names a multiword phrase and stays TR1
633-
# negative, matching the legacy trigger grammar.
634-
_DESCRIPTION_TRAILING_DISCOURSE_WORDS: set[str] = {
635-
"there",
636-
"here",
637-
"now",
638-
"then",
639-
}
640-
641628
# Bare universal-scope statements: the whole clause is a catch-all scope
642629
# ("all messages"), which the legacy trigger grammar also flags as TR3.
643630
_DESCRIPTION_BARE_SCOPE_RE = re.compile(
@@ -2256,17 +2243,11 @@ def _analyze_triggers(
22562243
phrase = phrase_match.group("phrase")
22572244
phrase_lower = phrase.lower()
22582245
phrase_words = phrase_lower.split()
2259-
# A broad word followed only by trailing discourse words names
2260-
# the broad word ("says hello there"); anything else multiword
2261-
# names a phrase and is not an overly broad single-word trigger.
2262-
broad_head_with_prose_tail = (
2263-
len(phrase_words) > 1
2264-
and phrase_words[0] in _OVERLY_BROAD_SINGLE_WORDS
2265-
and all(word in _DESCRIPTION_TRAILING_DISCOURSE_WORDS for word in phrase_words[1:])
2266-
)
2267-
if (
2268-
len(phrase_words) == 1 and phrase_lower in _OVERLY_BROAD_SINGLE_WORDS
2269-
) or broad_head_with_prose_tail:
2246+
# A multiword phrase names the complete bounded wording, even
2247+
# when its final word could also be read as discourse prose. In
2248+
# particular, "go there" and "work now" are valid phrases and
2249+
# must not be reduced to the broad heads "go" and "work".
2250+
if len(phrase_words) == 1 and phrase_lower in _OVERLY_BROAD_SINGLE_WORDS:
22702251
broad_word = phrase_words[0]
22712252
findings.append(
22722253
Finding(

‎tests/unit/test_patterns_new.py‎

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2513,24 +2513,16 @@ def test_description_broad_trigger_word_reaches_tr1(self) -> None:
25132513
assert len(tr1) == 1
25142514
assert "hello" in tr1[0].message
25152515

2516-
@pytest.mark.parametrize(
2517-
"trailing",
2518-
[
2519-
pytest.param("there", id="there"),
2520-
pytest.param("here", id="here"),
2521-
],
2522-
)
2523-
def test_description_broad_word_with_trailing_prose_reaches_tr1(self, trailing: str) -> None:
2524-
"""MohammedAlkindi #541: a broad word followed by trailing discourse
2525-
prose still names the broad word ("says hello there"), so TR1 fires.
2526-
Fixtures must not all end on the trigger phrase."""
2516+
@pytest.mark.parametrize("phrase", ["go there", "work now", "hello there", "hello here"])
2517+
def test_description_bounded_phrase_with_discourse_word_stays_negative(
2518+
self, phrase: str
2519+
) -> None:
2520+
"""#609: a discourse-looking tail must not shrink a phrase to its head."""
25272521
findings = sc_mod._analyze_triggers(
2528-
{"description": ("Use this skill whenever the user says hello " + trailing)},
2522+
{"description": f"Use this skill whenever the user says {phrase}"},
25292523
"myskill",
25302524
)
2531-
tr1 = [finding for finding in findings if finding.rule_id == "TR1"]
2532-
assert len(tr1) == 1
2533-
assert "activates on 'hello'" in tr1[0].message
2525+
assert not any(finding.rule_id == "TR1" for finding in findings), phrase
25342526

25352527
def test_description_article_not_skipped_as_filler(self) -> None:
25362528
"""MohammedAlkindi #541: bare articles are not filler words, so the

0 commit comments

Comments
 (0)