fix: Fuzzy matching fallback + CI/CD pipeline consolidation#114
Merged
fix: Fuzzy matching fallback + CI/CD pipeline consolidation#114
Conversation
…nstalled The fuzzy matching functions (compare_fuzzy, similarity_score, partial_ratio) relied on _MinimalFuzz which only returned binary 0/100 scores. When rapidfuzz is not installed, similar strings like "1.2.3" vs "1.2.4" incorrectly scored 0. Now checks USE_RAPIDFUZZ/USE_FUZZYWUZZY flags instead of truthy _fuzz object, and falls back to difflib.SequenceMatcher for proper fuzzy scoring. Also includes: - CI: Harden security step error handling in advanced-ci.yml - CI: Raise coverage threshold from 15% to 50% - CI: Replace fountainhead/action-wait-for-check with gh pr checks polling - Enhanced matching: Add SequenceMatcher fallback for fuzz-less environments Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
Reviewer's GuideImplements a more realistic fuzzy-matching fallback using difflib.SequenceMatcher when rapidfuzz/fuzzywuzzy are unavailable, adjusts tests to the new scoring behavior, adds a character-level fallback in enhanced_matching, and hardens CI workflows by making security scans non-blocking, renaming artifacts, increasing coverage thresholds, and replacing a deprecated Dependabot wait-for-check action with a gh pr checks polling loop. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
🔒 Security Analysis ReportSecurity Analysis ReportGenerated: Tue Mar 17 08:18:45 UTC 2026 Bandit Security ScanSafety Check ResultsPip-Audit Results |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
compare_fuzzy, the condition now only checksUSE_RAPIDFUZZ or USE_FUZZYWUZZYand then unconditionally calls_fuzz.ratio(...); consider mirroring the defensive pattern used insimilarity_score(e.g. checking_fuzzandhasattr(_fuzz, 'ratio')) so that tests or callers that patch_fuzztoNonedon’t hit anAttributeError. - In
enhanced_matching.pyyou addedSequenceMatcherusage incalculate_similaritybut did not add the correspondingfrom difflib import SequenceMatcherimport in that module, which will raise aNameErrorat runtime. - The new
gh pr checksloop independabot-auto-merge.ymlassumes the GitHub CLI is available; you may want to add an explicit installation/setup step or a guard that fails with a clear message ifghis missing, to avoid silent CI breakage in environments where it isn’t preinstalled.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `compare_fuzzy`, the condition now only checks `USE_RAPIDFUZZ or USE_FUZZYWUZZY` and then unconditionally calls `_fuzz.ratio(...)`; consider mirroring the defensive pattern used in `similarity_score` (e.g. checking `_fuzz` and `hasattr(_fuzz, 'ratio')`) so that tests or callers that patch `_fuzz` to `None` don’t hit an `AttributeError`.
- In `enhanced_matching.py` you added `SequenceMatcher` usage in `calculate_similarity` but did not add the corresponding `from difflib import SequenceMatcher` import in that module, which will raise a `NameError` at runtime.
- The new `gh pr checks` loop in `dependabot-auto-merge.yml` assumes the GitHub CLI is available; you may want to add an explicit installation/setup step or a guard that fails with a clear message if `gh` is missing, to avoid silent CI breakage in environments where it isn’t preinstalled.
## Individual Comments
### Comment 1
<location path=".github/workflows/dependabot-auto-merge.yml" line_range="63-68" />
<code_context>
- ref: ${{ github.event.pull_request.head.sha }}
- timeoutSeconds: 900
- intervalSeconds: 30
+ - name: Wait for CI and security checks
+ run: |
+ echo "Waiting for required checks to pass..."
+ # Use gh pr checks which respects branch protection required checks
+ for i in $(seq 1 60); do
+ STATUS=$(gh pr checks "$PR_URL" 2>&1 || true)
+ if echo "$STATUS" | grep -q "fail"; then
+ echo "Some checks failed:"
</code_context>
<issue_to_address>
**issue (bug_risk):** The `gh pr checks` output is treated as success for any case that doesn't contain the strings `fail` or `pending`, which can hide CLI/auth/config errors.
Because `gh pr checks "$PR_URL"` is wrapped in `|| true`, any CLI/auth/config error still yields a `STATUS` string. If that output doesn’t contain `fail` or `pending`, the loop will treat it as success and exit, so a broken `gh` setup or transient API issue could allow merges without truly validating checks.
It would be safer to treat `gh` failures and unexpected/empty output as failures—e.g., check the `gh` exit code explicitly, or use a known-success pattern (such as `gh pr checks --json` and checking status fields) and only consider recognized successful states as passing.
</issue_to_address>
### Comment 2
<location path=".github/workflows/advanced-ci.yml" line_range="161-164" />
<code_context>
+ echo '{"vulnerabilities":[]}' > safety-report.json
+ fi
- name: Run pip-audit
run: |
- pip-audit --desc --format json --output pip-audit-report.json || \
- (pip-audit --desc && echo '{"vulnerabilities":[]}' > pip-audit-report.json)
+ pip-audit --desc --format json --output pip-audit-report.json || true
+ pip-audit --desc || true
- name: Upload security reports
</code_context>
<issue_to_address>
**issue (bug_risk):** `pip-audit-report.json` may not exist when upload-artifact runs, causing the step to fail.
Both `pip-audit` commands now allow failure (`|| true`), but there’s no longer a fallback to ensure `pip-audit-report.json` exists. If the JSON run fails before writing the file (e.g., `pip-audit` missing, network error), `pip-audit-report.json` won’t be created and `actions/upload-artifact` will fail. Please add a minimal fallback (e.g., `echo '{
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12f14913c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| echo "Waiting for required checks to pass..." | ||
| # Use gh pr checks which respects branch protection required checks | ||
| for i in $(seq 1 60); do | ||
| STATUS=$(gh pr checks "$PR_URL" 2>&1 || true) |
There was a problem hiding this comment.
gh pr checks errors in wait loop
The polling step masks every gh pr checks failure with || true, so transient API/auth errors are treated as normal output and can fall through to the else branch that prints “All checks passed!” and proceeds to gh pr merge --auto. In security-auto-merge, this means a failed status fetch can bypass the intended gate and enqueue auto-merge without actually verifying CI/security results.
Useful? React with 👍 / 👎.
- Fix test patches to also set USE_RAPIDFUZZ/USE_FUZZYWUZZY flags (tests failed on CI where rapidfuzz is installed) - Remove duplicate security scanning from advanced-ci.yml (already handled by dedicated security.yml) - Fix Final Validation to handle skipped integration-tests gracefully - Fix coverage.yml retry threshold consistency (was 15%, now 50%) - Unpin TruffleHog from patch version v3.93.8 to major tag v3 - Unpin dependabot/fetch-metadata from v2.5.0 to v2 - Replace fragile grep-based CI check polling with gh pr checks --watch - Add artifact retention-days (14d for CI, 30d for releases/security) - Scope workflow triggers to master branch to prevent duplicate runs across ci.yml, lint.yml, coverage.yml, security.yml, performance.yml Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
docdyhr
changed the title
Workflow consolidation (10 → 8 workflows): - Merge advanced-ci.yml into ci.yml as single unified CI workflow with setup, quality, test-matrix, build, integration, performance, error-handling, docs, plugin, and final-validation jobs - Merge publish-pypi.yml into release.yml — single release pipeline with TestPyPI/PyPI publishing, sigstore signing, and GitHub Release attachment - Delete advanced-ci.yml and publish-pypi.yml Pipeline improvements: - Add concurrency groups to all workflows (cancel stale PR runs) - Use composite action (setup-python-deps) consistently across all jobs - Add post-release smoke tests: install from PyPI on all Python/OS combinations, verify version, test CLI and core imports - Add KeyboardInterrupt handling to test runner (from old ci.yml) - Scope lint/coverage/security/performance triggers to master branch README updates: - Update test count badge (1,136 → 2,158) - Add Release Pipeline workflow badge - Add PyPI package badge Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
docdyhr
changed the title
TruffleHog does not publish major version tags (v3), only full patch tags (v3.93.8). Dependabot handles automatic version bumps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
🔒 Security Analysis ReportSecurity Analysis ReportGenerated: Tue Mar 17 10:15:04 UTC 2026 Bandit Security ScanSafety Check ResultsPip-Audit Results |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fuzzy Matching Fix
compare_fuzzy,similarity_score, andpartial_rationow usedifflib.SequenceMatcheras fallback when rapidfuzz/fuzzywuzzy are not installedUSE_RAPIDFUZZ/USE_FUZZYWUZZYflags instead of truthy_fuzzobjectCI/CD Pipeline Consolidation (10 → 8 workflows)
ci.yml+advanced-ci.ymlci.ymlpublish-pypi.yml+release.ymlrelease.ymlPipeline Improvements
setup-python-deps) used consistently across all jobsmaster(CI workflow handles feature branches)v3.93.8→v3, dependabot/fetch-metadatav2.5.0→v2gh pr checks --watchREADME Updates
Test plan
Note on branch protection
The workflow file names changed (
advanced-ci.ymldeleted,ci.ymlrebuilt). If branch protection rules reference specific check names from the old workflows, they may need to be updated to the new check names.🤖 Generated with Claude Code