fix(merge-tracker): filter seniority+location stopwords and require o…#356
Open
darshan3131 wants to merge 2 commits intosantifer:mainfrom
Open
fix(merge-tracker): filter seniority+location stopwords and require o…#356darshan3131 wants to merge 2 commits intosantifer:mainfrom
darshan3131 wants to merge 2 commits intosantifer:mainfrom
Conversation
…verlap ratio in roleFuzzyMatch (santifer#329)
📝 WalkthroughWalkthroughRefines role string comparison logic in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@merge-tracker.mjs`:
- Around line 105-121: The roleFuzzyMatch function is too strict with its
current hard requirement overlap >= 2; modify roleFuzzyMatch (and use
roleTokens) so that the overlap threshold is conditional on the smaller token
count: compute wordsA, wordsB, minLen and ratio as before, but require overlap
>= 2 only when minLen >= 3; otherwise accept overlap >= 1 provided the ratio >=
0.6 (or when minLen === 1 require ratio === 1 to avoid weak single-token
matches). Update the return logic to reflect these branches and add unit tests
for the example pairs ("Backend Engineer" vs "Backend Developer", "Frontend
Engineer" vs "Frontend Developer", "Senior Engineer" vs "Engineer", "Software
Engineer" vs "Software Developer") to ensure deduplication behaves as expected.
- Around line 97-103: roleTokens currently drops tokens shorter than 4
characters which removes valid role signals (e.g. qa, ios, api) and causes
roleFuzzyMatch to undercount overlap; change the filter in roleTokens (function
roleTokens) to allow 2–3 character tokens (e.g. use length >= 2) or implement a
whitelist of short role tokens to retain, ensuring the stopword set remains
applied so noisy short words are still removed; update any tests or callers
relying on roleTokens (and re-evaluate roleFuzzyMatch overlap logic if needed)
to prevent duplicate entries in applications.md.
- Around line 76-95: ROLE_STOPWORDS contains a duplicate 'intern' entry; remove
the redundant string so the set literal is deduplicated (edit the ROLE_STOPWORDS
array used to construct the Set), or replace the literal with a deduplicated
source (e.g., build the array and run Array.from(new Set(...)) before passing
into the Set) to ensure each stopword appears only once; refer to the
ROLE_STOPWORDS constant in this module when making the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 71abb765-ac43-4835-a987-e2613f0e2e0b
📒 Files selected for processing (1)
merge-tracker.mjs
Comment on lines
+76
to
+95
| const ROLE_STOPWORDS = new Set([ | ||
| // seniority / level | ||
| 'junior', 'mid', 'middle', 'senior', 'staff', 'principal', 'lead', 'head', | ||
| 'chief', 'associate', 'intern', 'entry', 'level', | ||
| // contract / mode | ||
| 'remote', 'hybrid', 'onsite', 'contract', 'contractor', 'freelance', | ||
| 'fulltime', 'parttime', 'permanent', 'temporary', 'intern', 'internship', | ||
| // generic job words | ||
| 'role', 'position', 'opportunity', 'team', 'based', | ||
| // very common locations (extend in portals.yml later if needed) | ||
| 'bangalore', 'bengaluru', 'mumbai', 'delhi', 'hyderabad', 'pune', 'chennai', | ||
| 'london', 'berlin', 'paris', 'madrid', 'barcelona', 'amsterdam', 'dublin', | ||
| 'york', 'francisco', 'seattle', 'boston', 'austin', 'chicago', 'toronto', | ||
| 'tokyo', 'singapore', 'sydney', 'melbourne', 'lisbon', 'warsaw', | ||
| // regions / countries | ||
| 'europe', 'emea', 'apac', 'latam', 'americas', 'india', 'spain', 'germany', | ||
| 'france', 'italy', 'canada', 'brazil', 'mexico', 'japan', | ||
| // prepositions leaking through length filter | ||
| 'with', 'from', 'into', 'over', 'this', 'that', | ||
| ]); |
There was a problem hiding this comment.
Duplicate 'intern' in ROLE_STOPWORDS.
'intern' appears on both line 79 (seniority block) and line 82 (contract block). Harmless in a Set but indicates the list wasn't deduplicated.
🧹 Proposed fix
// contract / mode
'remote', 'hybrid', 'onsite', 'contract', 'contractor', 'freelance',
- 'fulltime', 'parttime', 'permanent', 'temporary', 'intern', 'internship',
+ 'fulltime', 'parttime', 'permanent', 'temporary', 'internship',🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@merge-tracker.mjs` around lines 76 - 95, ROLE_STOPWORDS contains a duplicate
'intern' entry; remove the redundant string so the set literal is deduplicated
(edit the ROLE_STOPWORDS array used to construct the Set), or replace the
literal with a deduplicated source (e.g., build the array and run Array.from(new
Set(...)) before passing into the Set) to ensure each stopword appears only
once; refer to the ROLE_STOPWORDS constant in this module when making the
change.
Comment on lines
+97
to
+103
| function roleTokens(s) { | ||
| return s | ||
| .toLowerCase() | ||
| .replace(/[^a-z0-9\s]/g, ' ') | ||
| .split(/\s+/) | ||
| .filter(w => w.length > 3 && !ROLE_STOPWORDS.has(w)); | ||
| } |
There was a problem hiding this comment.
length > 3 filter discards meaningful short role tokens.
Common role signals are 2–3 characters and get silently stripped: qa, ml, ai, ux, ui, ios, sre, dev, web, api, aws, gcp, sql, php, go. Combined with the new overlap >= 2 requirement in roleFuzzyMatch, roles like "QA Engineer" vs "QA Engineer" reduce to {engineer} on both sides (overlap = 1) and fail to match — producing duplicate entries in applications.md.
Consider lowering the threshold to length >= 2 (the stopword set already filters the noisy short words like mid, lead, etc.), or whitelisting a set of known short role tokens.
♻️ Proposed fix
- .filter(w => w.length > 3 && !ROLE_STOPWORDS.has(w));
+ .filter(w => w.length >= 2 && !ROLE_STOPWORDS.has(w));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function roleTokens(s) { | |
| return s | |
| .toLowerCase() | |
| .replace(/[^a-z0-9\s]/g, ' ') | |
| .split(/\s+/) | |
| .filter(w => w.length > 3 && !ROLE_STOPWORDS.has(w)); | |
| } | |
| function roleTokens(s) { | |
| return s | |
| .toLowerCase() | |
| .replace(/[^a-z0-9\s]/g, ' ') | |
| .split(/\s+/) | |
| .filter(w => w.length >= 2 && !ROLE_STOPWORDS.has(w)); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@merge-tracker.mjs` around lines 97 - 103, roleTokens currently drops tokens
shorter than 4 characters which removes valid role signals (e.g. qa, ios, api)
and causes roleFuzzyMatch to undercount overlap; change the filter in roleTokens
(function roleTokens) to allow 2–3 character tokens (e.g. use length >= 2) or
implement a whitelist of short role tokens to retain, ensuring the stopword set
remains applied so noisy short words are still removed; update any tests or
callers relying on roleTokens (and re-evaluate roleFuzzyMatch overlap logic if
needed) to prevent duplicate entries in applications.md.
Comment on lines
105
to
121
| function roleFuzzyMatch(a, b) { | ||
| const wordsA = a.toLowerCase().split(/\s+/).filter(w => w.length > 3); | ||
| const wordsB = b.toLowerCase().split(/\s+/).filter(w => w.length > 3); | ||
| const overlap = wordsA.filter(w => wordsB.some(wb => wb.includes(w) || w.includes(wb))); | ||
| return overlap.length >= 2; | ||
| const wordsA = roleTokens(a); | ||
| const wordsB = roleTokens(b); | ||
| if (wordsA.length === 0 || wordsB.length === 0) return false; | ||
|
|
||
| const setB = new Set(wordsB); | ||
| const overlap = wordsA.filter(w => setB.has(w)).length; | ||
| if (overlap === 0) return false; | ||
|
|
||
| // Jaccard-style ratio on content tokens. Two roles are "the same" only | ||
| // when the overlap dominates the smaller side — not when they just share | ||
| // a location + "engineer". | ||
| const minLen = Math.min(wordsA.length, wordsB.length); | ||
| const ratio = overlap / minLen; | ||
|
|
||
| return overlap >= 2 && ratio >= 0.6; | ||
| } |
There was a problem hiding this comment.
overlap >= 2 is too strict for short role strings and will under-dedupe.
Many legitimate duplicates collapse to a single signal token per side after stopword filtering, so they can never reach overlap >= 2:
"Senior Software Engineer"vs"Software Engineer"→{software, engineer}both sides → overlap = 2 ✅ (ok)"Backend Engineer"vs"Backend Developer"→{backend, engineer}vs{backend, developer}→ overlap = 1 ❌"Frontend Engineer"vs"Frontend Developer"→ overlap = 1 ❌"Senior Engineer"vs"Engineer"→{engineer}vs{engineer}→ overlap = 1 ❌"Software Engineer"vs"Software Developer"→ overlap = 1 ❌
The dedup path at line 307 will now miss these and write duplicate rows into applications.md. Options:
- Require
overlap >= 2only whenmin(|wordsA|, |wordsB|) >= 3; otherwise acceptoverlap >= 1withratio >= 0.6(orratio == 1on the smaller side). - Treat
engineer/developer(and similardeveloper|engineer|programmer) as synonyms before overlap.
♻️ Suggested adjustment
- const minLen = Math.min(wordsA.length, wordsB.length);
- const ratio = overlap / minLen;
-
- return overlap >= 2 && ratio >= 0.6;
+ const minLen = Math.min(wordsA.length, wordsB.length);
+ const ratio = overlap / minLen;
+
+ // For very short token sets (1–2 signal tokens per side), a single full
+ // overlap is meaningful; only require >=2 when both sides are richer.
+ const minOverlap = minLen >= 3 ? 2 : 1;
+ return overlap >= minOverlap && ratio >= 0.6;Worth adding a couple of unit tests covering the pairs above to lock this behavior in.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function roleFuzzyMatch(a, b) { | |
| const wordsA = a.toLowerCase().split(/\s+/).filter(w => w.length > 3); | |
| const wordsB = b.toLowerCase().split(/\s+/).filter(w => w.length > 3); | |
| const overlap = wordsA.filter(w => wordsB.some(wb => wb.includes(w) || w.includes(wb))); | |
| return overlap.length >= 2; | |
| const wordsA = roleTokens(a); | |
| const wordsB = roleTokens(b); | |
| if (wordsA.length === 0 || wordsB.length === 0) return false; | |
| const setB = new Set(wordsB); | |
| const overlap = wordsA.filter(w => setB.has(w)).length; | |
| if (overlap === 0) return false; | |
| // Jaccard-style ratio on content tokens. Two roles are "the same" only | |
| // when the overlap dominates the smaller side — not when they just share | |
| // a location + "engineer". | |
| const minLen = Math.min(wordsA.length, wordsB.length); | |
| const ratio = overlap / minLen; | |
| return overlap >= 2 && ratio >= 0.6; | |
| } | |
| function roleFuzzyMatch(a, b) { | |
| const wordsA = roleTokens(a); | |
| const wordsB = roleTokens(b); | |
| if (wordsA.length === 0 || wordsB.length === 0) return false; | |
| const setB = new Set(wordsB); | |
| const overlap = wordsA.filter(w => setB.has(w)).length; | |
| if (overlap === 0) return false; | |
| // Jaccard-style ratio on content tokens. Two roles are "the same" only | |
| // when the overlap dominates the smaller side — not when they just share | |
| // a location + "engineer". | |
| const minLen = Math.min(wordsA.length, wordsB.length); | |
| const ratio = overlap / minLen; | |
| // For very short token sets (1–2 signal tokens per side), a single full | |
| // overlap is meaningful; only require >=2 when both sides are richer. | |
| const minOverlap = minLen >= 3 ? 2 : 1; | |
| return overlap >= minOverlap && ratio >= 0.6; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@merge-tracker.mjs` around lines 105 - 121, The roleFuzzyMatch function is too
strict with its current hard requirement overlap >= 2; modify roleFuzzyMatch
(and use roleTokens) so that the overlap threshold is conditional on the smaller
token count: compute wordsA, wordsB, minLen and ratio as before, but require
overlap >= 2 only when minLen >= 3; otherwise accept overlap >= 1 provided the
ratio >= 0.6 (or when minLen === 1 require ratio === 1 to avoid weak
single-token matches). Update the return logic to reflect these branches and add
unit tests for the example pairs ("Backend Engineer" vs "Backend Developer",
"Frontend Engineer" vs "Frontend Developer", "Senior Engineer" vs "Engineer",
"Software Engineer" vs "Software Developer") to ensure deduplication behaves as
expected.
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.
…verlap ratio in roleFuzzyMatch (#329)
What does this PR do?
Related issue
Type of change
Checklist
node test-all.mjsand all tests passQuestions? Join the Discord for faster feedback.
Summary by CodeRabbit