Skip to content

feat(secrets): add Dedupe to remove subsumed patterns - #629

Open
joe0BAB wants to merge 1 commit into
mainfrom
feat/proto
Open

feat(secrets): add Dedupe to remove subsumed patterns#629
joe0BAB wants to merge 1 commit into
mainfrom
feat/proto

Conversation

@joe0BAB

@joe0BAB joe0BAB commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Dedupe collapses a pattern list to its maximal elements: patterns whose matches are fully covered by another entry are dropped, equivalent spellings (e.g. *, / and */) keep their first occurrence, and input order is preserved.

Containment is decided as true language containment over identifiers, computed on canonical token sequences with an O(L^2) DP per pair; length, shape and literal-anchor filters skip most pairs. This is deliberately stricter than Pattern.Includes, which also treats a literal as covering '' and '' as covering '**'.

Verified against a brute-force oracle: every pattern pair with up to three components, checked against all identifiers with up to nine components over a three-symbol alphabet.

@joe0BAB
joe0BAB marked this pull request as ready for review August 20, 2026 13:52

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assessment: 🟢 APPROVE

No bugs found in the changed code. The Dedupe function, canonicalize, and includes DP are all correct.

Summary: This PR introduces a well-implemented pattern deduplication algorithm. Key areas checked:

  • canonicalize: Correctly collapses adjacent wildcards — stars and gap accumulate independently and are emitted together in flush(). Stars before ** are not lost; they stay in stars and are emitted first. Patterns like **/*, */**, and ** all correctly collapse to the same [*, **] canonical form.
  • includes DP: The two-row rolling array (prev/cur) is fully overwritten on every outer iteration (both cur[nq] at the top and all cur[j] in the inner loop), so no stale values bleed through. The q[j]=tokenGap case formula p[i].kind==tokenStar && cur[j+1] && prev[j] correctly requires both: gap=0 expansion (cur[j+1]) and gap≥1 expansion (prev[j]), and correctly rejects literals since they can't cover all gap expansions.
  • Dedupe loop: The mutual-inclusion tie-breaking (keep earlier entry when j < i, keep i when mutual) is sound. Transitivity ensures that even if a dominator j is itself later dominated, i is still correctly dropped.
  • subsumes filters: firstLit/lastLit are correctly "" for wildcard-anchored entries (zero value of string), so the literal-anchor filters only fire when other truly starts/ends with a literal.
  • Tests: The exhaustive oracle in TestDedupeExhaustive (checking all 84 patterns against all identifiers up to 9 components) provides strong correctness evidence.

@joe0BAB
joe0BAB force-pushed the feat/proto branch 2 times, most recently from 28e1dd2 to 86dbf27 Compare August 20, 2026 14:07
Dedupe collapses a pattern list to its maximal elements: patterns whose
matches are fully covered by another entry are dropped, equivalent
spellings (e.g. **, **/* and */**) keep their first occurrence, and
input order is preserved.

Containment is decided as true language containment over identifiers,
computed on canonical token sequences with an O(L^2) DP per pair;
length, shape and literal-anchor filters skip most pairs. This is
deliberately stricter than Pattern.Includes, which also treats a
literal as covering '*' and '*' as covering '**'.

Verified against a brute-force oracle: every pattern pair with up to
three components, checked against all identifiers with up to nine
components over a three-symbol alphabet.

Signed-off-by: Johannes Großmann <grossmann.johannes@t-online.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants