fix(updater): add missing language modes and scripts to SYSTEM_PATHS …#353
fix(updater): add missing language modes and scripts to SYSTEM_PATHS …#353darshan3131 wants to merge 3 commits intosantifer:mainfrom
Conversation
📝 WalkthroughWalkthroughThe PR expands the Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related issues
🚥 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 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to career-ops, @darshan3131! Thanks for your first PR.
A few things to know:
- Tests will run automatically — check the status below
- Make sure you've linked a related issue (required for features)
- Read CONTRIBUTING.md if you haven't
We'll review your PR soon. Join our Discord if you have questions.
darshan3131
force-pushed
the
fix/337-system-paths
branch
from
5f36eff to
f2b1ffa
Compare
darshan3131
force-pushed
the
fix/337-system-paths
branch
from
f2b1ffa to
5f36eff
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DATA_CONTRACT.md (1)
44-51:⚠️ Potential issue | 🟡 MinorMissing
modes/interview-prep.mdentry.
update-system.mjsaddsmodes/interview-prep.mdtoSYSTEM_PATHS, but the System Layer table here only listsmodes/patterns.mdandmodes/followup.md. Since this PR is explicitly about reconciling the updater allowlist with the documented system/user boundary, please also documentmodes/interview-prep.mdhere so the contract matches the updater.Note: this is the file
modes/interview-prep.md, distinct from the user-layerinterview-prep/story-bank.mdalready listed above.📝 Proposed addition
| `modes/patterns.md` | Pattern analysis instructions | | `modes/followup.md` | Follow-up cadence instructions | +| `modes/interview-prep.md` | Interview prep instructions | | `modes/de/*` | German language modes |🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@DATA_CONTRACT.md` around lines 44 - 51, The System Layer table in DATA_CONTRACT.md is missing an entry for modes/interview-prep.md even though update-system.mjs adds it to SYSTEM_PATHS; update the System Layer table to include a row for `modes/interview-prep.md` with a short description like "Interview preparation instructions" so the documented contract matches the updater allowlist (refer to update-system.mjs and the SYSTEM_PATHS constant to confirm the file name).
🤖 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 76-95: ROLE_STOPWORDS contains duplicate 'intern' and an
aggressive location/region block that can over-filter legitimate role/company
signals; update ROLE_STOPWORDS to remove duplicates and consider removing or
pruning the city/region entries (or move them to portals.yml) and then modify
the tokenization in roleTokens (the length filter) to allow short domain tokens
by changing the length check from >3 to >=2 or by introducing a shortToken
allowlist (e.g., ['ai','ml','pm','se','eng','sa']) so tokens like "AI", "PM",
etc. are preserved; ensure roleFuzzyMatch still gets non-empty tokens and keep
company matching responsibility in normalizeCompany so you don't need broad
location stopwords for deduplication.
---
Outside diff comments:
In `@DATA_CONTRACT.md`:
- Around line 44-51: The System Layer table in DATA_CONTRACT.md is missing an
entry for modes/interview-prep.md even though update-system.mjs adds it to
SYSTEM_PATHS; update the System Layer table to include a row for
`modes/interview-prep.md` with a short description like "Interview preparation
instructions" so the documented contract matches the updater allowlist (refer to
update-system.mjs and the SYSTEM_PATHS constant to confirm the file name).
🪄 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: b5a7a8c0-267d-422f-a827-ecce53504fe0
📒 Files selected for processing (6)
DATA_CONTRACT.mdVERSIONmerge-tracker.mjsmodes/scan.mdpackage.jsonupdate-system.mjs
| 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.
Stopword list risks over-filtering legitimate role signal.
A few concerns worth verifying before release:
'intern'is listed twice (lines 79 and 82). Harmless in aSet, but indicates the list wasn't reviewed carefully.- Dropping single city tokens like
'york','francisco','boston','paris'is intentional for location-in-title cases, but this also strips company/product-family signals if a role string contains them for legitimate reasons. Given the length filter already requires>3chars and company is matched separately vianormalizeCompany, you may not need the location/region block at all for dedup. - The combined
length > 3+ stopword filter can drop roles down to 0 or 1 tokens (e.g."AI PM Remote"→[]becauseai/pmfail the length filter andremoteis a stopword).roleTokensthen returns[], soroleFuzzyMatchreturnsfalseand the duplicate is silently missed. Consider lowering the length filter to>=2or keeping short domain tokens (ai,ml,pm,sa,se,eng) on an allowlist.
🤖 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 duplicate
'intern' and an aggressive location/region block that can over-filter legitimate
role/company signals; update ROLE_STOPWORDS to remove duplicates and consider
removing or pruning the city/region entries (or move them to portals.yml) and
then modify the tokenization in roleTokens (the length filter) to allow short
domain tokens by changing the length check from >3 to >=2 or by introducing a
shortToken allowlist (e.g., ['ai','ml','pm','se','eng','sa']) so tokens like
"AI", "PM", etc. are preserved; ensure roleFuzzyMatch still gets non-empty
tokens and keep company matching responsibility in normalizeCompany so you don't
need broad location stopwords for deduplication.
…antifer#337) SYSTEM_PATHS only listed modes/de/ and a subset of root scripts. It silently skipped modes/fr, modes/ja, modes/pt, modes/ru and newer mode files (patterns, followup, interview-prep), plus root scripts shipped since v1.0: scan.mjs, check-liveness.mjs, liveness-core.mjs, analyze-patterns.mjs, followup-cadence.mjs, doctor.mjs, test-all.mjs. As a result, 'npm run update' never pulled these files on user installs — any fix to them was dead on arrival. Added all missing paths. User-layer paths are unchanged.
darshan3131
force-pushed
the
fix/337-system-paths
branch
from
5f36eff to
1854c7d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
update-system.mjs (1)
305-313:⚠️ Potential issue | 🟡 MinorRollback amplifies a pre-existing
git addhazard for newly listed paths.
rollback()callsaddPaths(SYSTEM_PATHS)unconditionally (Line 313), unlikeapply()which only stages successfully-updated paths. For users rolling back from a version that never hadmodes/fr/,modes/ja/,modes/pt/,modes/ru/,scan.mjs,check-liveness.mjs, etc. locally, the per-path checkout on Line 307 silently fails and the path remains absent — thengit add -- modes/ru/will fail with "did not match any files", aborting the rollback commit and leaving the repo in a partially-restored state.This PR doesn't introduce the pattern, but it materially widens its blast radius since the new entries are exactly the ones most likely to be missing on older installs.
♻️ Suggested fix: mirror apply()'s pattern and only stage paths that actually restored
const latest = branchList[0]; console.log(`Rolling back to: ${latest}`); // Checkout system files from backup branch + const restored = []; for (const path of SYSTEM_PATHS) { try { git('checkout', latest, '--', path); + restored.push(path); } catch { // File may not have existed in backup } } - addPaths(SYSTEM_PATHS); + addPaths(restored); git('commit', '-m', `chore: rollback system files from ${latest}`);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@update-system.mjs` around lines 305 - 313, The rollback() function unconditionally calls addPaths(SYSTEM_PATHS) which reintroduces the "git add" failure when per-path git checkout silently skipped missing files; change rollback() to mirror apply() by tracking which paths actually restored (e.g., after each git('checkout', latest, '--', path) inside the try block push path into a restoredPaths array) and then call addPaths(restoredPaths) instead of addPaths(SYSTEM_PATHS), ensuring only successfully-checked-out paths are staged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@update-system.mjs`:
- Around line 305-313: The rollback() function unconditionally calls
addPaths(SYSTEM_PATHS) which reintroduces the "git add" failure when per-path
git checkout silently skipped missing files; change rollback() to mirror apply()
by tracking which paths actually restored (e.g., after each git('checkout',
latest, '--', path) inside the try block push path into a restoredPaths array)
and then call addPaths(restoredPaths) instead of addPaths(SYSTEM_PATHS),
ensuring only successfully-checked-out paths are staged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 72e6ddb9-52b4-4b67-a758-841a3d7b4801
📒 Files selected for processing (1)
update-system.mjs
1 participant
…(#337)
SYSTEM_PATHS only listed modes/de/ and a subset of root scripts. It silently skipped modes/fr, modes/ja, modes/pt, modes/ru and newer mode files (patterns, followup, interview-prep), plus root scripts shipped since v1.0: scan.mjs, check-liveness.mjs, liveness-core.mjs, analyze-patterns.mjs, followup-cadence.mjs, doctor.mjs, test-all.mjs.
As a result, 'npm run update' never pulled these files on user installs — any fix to them was dead on arrival.
Added all missing paths. User-layer paths are unchanged.
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