fix(npm-registry): cherry-pick npm engine + maxResults:0 guard from #403 - #481
fix(npm-registry): cherry-pick npm engine + maxResults:0 guard from #403#481fuleinist wants to merge 6 commits into
Conversation
Adds NpmRegistryEngine using npm's public search API (registry.npmjs.org/-/v1/search), registered as a secondary engine in the code vertical alongside crates-io. Includes unit tests and updates the code-vertical engine-set assertions. Closes KnockOutEZ#144.
…arse, canonical URLs - register npm-registry as high tier in ENGINE_QUALITY so the vertical/registry consistency test passes - send descriptive wigolo/0.1 User-Agent header, matching crates-io adapter - Array.isArray guard on objects payload; cap maxResults after mapping valid packages so nameless rows don't count - construct npmjs URL from package name instead of trusting links.npm
coderabbitai finding on PR#403: data.objects dereference without validating data itself. If the registry returns null or a primitive (e.g. 'unexpected string', 42), accessing data.objects throws a TypeError instead of returning an empty result set. - Cast response.json() to unknown before narrowing - Add null + typeof object checks before reading .objects - Add tests for null and primitive top-level JSON payloads
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review. 📝 WalkthroughWalkthroughThe change adds an npm registry search engine. It maps npm package metadata to search results, validates API responses, handles limits and errors, and registers the engine as a high-quality secondary engine in the code vertical. Changesnpm Registry Search
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This change adds a localized guard so npm registry searches return no results when maxResults is zero, with a regression test covering the behavior. No actionable merge-blocking risk remains beyond normal checks and review. Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Caller
participant NpmRegistryEngine
participant NpmSearchAPI
Caller->>NpmRegistryEngine: Search for a package query
NpmRegistryEngine->>NpmSearchAPI: Request encoded query
NpmSearchAPI-->>NpmRegistryEngine: Return package metadata
NpmRegistryEngine-->>Caller: Return mapped search results
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
|
Ping @KnockOutEZ — this PR (#481) is a cherry-pick of the maxResults:0 guard from #403, with all CodeRabbit findings addressed and CI passing. It's the minimal fix needed before #403 can be merged properly. Any chance you could take a look? |
1 participant
Summary
Cherry-picks the npm registry engine + maxResults:0 guard from fix/maxresults-zero onto main, addressing the valid coderabbitai finding on PR #261 that
parseObjects()would return one result whenmaxResults: 0.Fix:
parseObjects()now returns[]early whenmaxResults <= 0(line 79).Context
PR #261 introduced the npm registry engine but the coderabbitai pre-merge check flagged that the
maxResults: 0edge case was not handled — the cap check happened after the first valid package was pushed. This PR applies the same fix stack to the main branch directly, resolving the finding.All 6 commits are included to ensure the npm registry engine lands cleanly on main with all the review-motivated fixes (UA header, quality tier, null guards).
Test Plan
maxResults: 0returns empty arraySummary by CodeRabbit
New Features
Bug Fixes