Skip to content

sequence has no way to count occurrences of the whole pattern within a scope, only occurrence does, and occurrence has no tag support #1161

Description

@theredspoon

Problem

sequence matches a tagged multi-token pattern but alerts once per match, no threshold/count concept. occurrence has a count threshold (Max/Min) but only matches a raw regex, no tag/upos field at all. Neither can count occurrences of a tagged pattern within a scope.

Motivating case

tbhb/vale-ai-tells ships VerbTricolonDensity.yml, an occurrence rule (max: 1) meant to flag more than one AI-cliché verb tricolon per paragraph. Its regex can't require the matched words be verbs, so it fires on plain noun-phrase lists too. The single-instance version of the same rule has an unmerged sequence-based fix using real POS tags. The density version can't get the same fix today: no way to express "this tagged pattern needs to occur 2+ times in one paragraph."

Root cause

NewSequence unconditionally rewrites every declared scope to sentence-level via sentenceScope(). Run() is invoked once per sentence regardless of what a rule declares. scope: paragraph on a sequence rule currently has no effect, it collapses to the same "every sentence, everywhere" behavior as an undeclared scope.

Practical effect: a paragraph with two real matches split across two sentences produces zero density alerts, since each sentence individually has only one match, at or under any reasonable threshold.

Proposed fix

Contained to sequence.go:

  • When a rule opts into count-threshold behavior, skip the sentenceScope() rewrite. Run() is then called once per the rule's actually-declared scope (e.g. once per paragraph).
  • Tag the whole block's text as today. This is already safe: the tagging function segments and tags per sentence internally regardless of how much text it's handed, so nothing degrades from seeing a whole paragraph instead of one sentence.
  • Add a lightweight pass mapping each tagged word to a sentence index (existing sentence tokenizer).
  • Add one guard in the match-walk (sequenceMatches) rejecting a match that spans two different sentences. Without it, a comma ending one sentence and a verb starting the next could be treated as adjacent.
  • Threshold/message logic mirrors occurrence's existing Max/Min convention (core.CondSprintf for the count), it just now runs over a correctly-scoped, boundary-guarded match set.

Alternative considered: extending occurrence.go

occurrence already runs once per a rule's real declared scope correctly, including working scope: raw, and could reuse sequence's tag-matching helpers directly (same package).

Set aside: occurrence runs concurrently across rules matching one block; sequence is deliberately excluded from concurrent execution because its tagging cache isn't synchronized (confirmed with Go's race detector, not just by reading the code). Fixable, but it touches a second file plus shared dispatch infrastructure other check types rely on, and gives Occurrence two structurally different, mutually exclusive matching modes. The sequence.go-only fix has a smaller blast radius.

Related work

This request is the remaining piece: counting the whole pattern within its real declared scope, not one token within it.

Status

Draft PR #1162 implements an earlier version of this design that predates the root-cause finding above. Happy to update it to match if this shape looks right.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions