Conversation
Each registry exposes README content differently — jsdelivr CDN for npm, dedicated API endpoint for crates.io, project pages for PyPI/RubyGems/Packagist. Adds readme(name, version?) to the URLBuilder interface and implements it across all five adapters. Also adds resolveReadmeUrl() helper for consumers who just want the URL from a Package object.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Helper as resolveReadmeUrl
participant Registry as Registry.urls()
participant External as Registry CDN/API
Client->>Helper: resolveReadmeUrl(pkg, urls, version?)
Helper->>Registry: urls.readme(pkg.name, version?)
Registry->>External: construct/readme URL (e.g., jsdelivr, crates API, package page)
External-->>Registry: (URL or content)
Registry-->>Helper: readme URL
Helper-->>Client: readme URL
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
No issues found across 11 files
Architecture diagram
sequenceDiagram
participant App as Consumer Application
participant Helpers as Helper Utilities
participant Builder as URLBuilder (Interface)
participant Npm as NpmRegistry
participant Cargo as CargoRegistry
participant Others as PyPI/Gems/Packagist
Note over App,Others: README URL Resolution Flow
App->>Helpers: resolveReadmeUrl(pkg, builder, version?)
Helpers->>Builder: NEW: readme(name, version?)
alt Ecosystem: npm
Builder->>Npm: readme(name, version)
Note right of Npm: Points to jsdelivr CDN
Npm-->>Helpers: "https://cdn.jsdelivr.net/npm/..."
else Ecosystem: cargo
Builder->>Cargo: readme(name, version)
Note right of Cargo: Points to crates.io API
Cargo-->>Helpers: "https://crates.io/api/v1/..."
else Ecosystem: PyPI / RubyGems / Packagist
Builder->>Others: readme(name, version)
Note right of Others: Points to project landing page
Others-->>Helpers: "https://{registry}/{pkg}..."
end
Helpers-->>App: Return specific README URL string
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/helpers.ts (1)
115-117: Default topkg.latestVersionwhenversionis omitted.Right now this ignores available package version context and can return floating README URLs.
💡 Proposed refinement
export function resolveReadmeUrl(pkg: Package, urls: URLBuilder, version?: string): string { - return urls.readme(pkg.name, version) + const resolvedVersion = version ?? (pkg.latestVersion || undefined) + return urls.readme(pkg.name, resolvedVersion) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/helpers.ts` around lines 115 - 117, The resolveReadmeUrl function currently ignores package context when version is omitted; update resolveReadmeUrl (and its call to urls.readme) to default the version to pkg.latestVersion when version is undefined or falsy (e.g., use a fallback like version ?? pkg.latestVersion or version || pkg.latestVersion) so the returned README URL is tied to the package's latestVersion from the Package object.test/unit/cached-registry.test.ts (1)
55-55: Add a passthrough assertion forurls().readme()in the URLs test.You updated the mock surface, but the cached wrapper behavior for
readmeis still untested; one assertion here would prevent regressions.Suggested test addition
describe('urls', () => { it('returns inner registry URLs', () => { const inner = createMockRegistry('npm') const cached = new CachedRegistry(inner) const urls = cached.urls() expect(urls.registry('pkg')).toBe('https://npm.example.com') + expect(urls.readme('pkg', '1.0.0')).toBe('https://npm.example.com/readme') }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/cached-registry.test.ts` at line 55, Add a passthrough assertion ensuring the cached wrapper forwards readme URL unchanged: in the URLs test that exercises urls() on the cached registry (look for the test referencing urls() and the mocked readme: () => `https://${ecosystem}.example.com/readme`), call urls().readme() and assert it equals the original mock value so the cached wrapper's readme passthrough behavior is covered and won't regress.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/registries/cargo.ts`:
- Around line 275-279: The current readme function returns an unversioned
crates.io README URL which 404s; update the readme(name: string, version?:
string) logic so that if version is omitted it first fetches crate metadata from
the crates.io API (e.g., GET /api/v1/crates/{name}) to determine the latest
version (use max_version or the first versions entry) and then construct and
return the versioned README URL /api/v1/crates/{name}/{version}/readme;
alternatively, make the version parameter non-optional on readme to force
callers to provide a version and remove the unversioned branch—also ensure you
handle and surface fetch errors cleanly.
---
Nitpick comments:
In `@src/helpers.ts`:
- Around line 115-117: The resolveReadmeUrl function currently ignores package
context when version is omitted; update resolveReadmeUrl (and its call to
urls.readme) to default the version to pkg.latestVersion when version is
undefined or falsy (e.g., use a fallback like version ?? pkg.latestVersion or
version || pkg.latestVersion) so the returned README URL is tied to the
package's latestVersion from the Package object.
In `@test/unit/cached-registry.test.ts`:
- Line 55: Add a passthrough assertion ensuring the cached wrapper forwards
readme URL unchanged: in the URLs test that exercises urls() on the cached
registry (look for the test referencing urls() and the mocked readme: () =>
`https://${ecosystem}.example.com/readme`), call urls().readme() and assert it
equals the original mock value so the cached wrapper's readme passthrough
behavior is covered and won't regress.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 266b01f1-4712-417b-be36-49f4f69b2f15
📒 Files selected for processing (11)
src/core/types.tssrc/helpers.tssrc/index.tssrc/registries/cargo.tssrc/registries/npm.tssrc/registries/packagist.tssrc/registries/pypi.tssrc/registries/rubygems.tstest/unit/cached-registry.test.tstest/unit/helpers.test.tstest/unit/registry.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (11)
src/registries/*.ts
📄 CodeRabbit inference engine (src/AGENTS.md)
Implement ecosystem adapters in
src/registries/*.tsand register through factory with side-effect import hub
src/registries/*.ts: Each registry adapter must exposeecosystem,fetchPackage,fetchVersions,fetchDependencies,fetchMaintainers, andurlsexports
Convert source-specific fields from upstream APIs into corePackage,Version,Dependency, andMaintainershapes
Map remote API failures to core error classes rather than passing through raw upstream errors
UseClientfor HTTP requests instead of callingfetchdirectly
Do not return raw upstream API payloads through public methods; always normalize to core types before exposing
Keep adapter internals self-contained with no cross-adapter imports
Isolate per-registry quirks and API-specific logic to individual adapter files; normalize before returning shared typesImplement
Registryinterface and register via side-effect import for new ecosystem support
Files:
src/registries/npm.tssrc/registries/pypi.tssrc/registries/rubygems.tssrc/registries/packagist.tssrc/registries/cargo.ts
src/**/*.ts
📄 CodeRabbit inference engine (src/AGENTS.md)
src/**/*.ts: Use.tsimport suffixes consistently throughout the codebase
Do not implement fetch/retry behavior outsidesrc/core/client.ts; all fetch logic must be centralized
src/**/*.ts: ESM-only; TypeScript imports keep.tsextension style in source
Do not duplicate retry/backoff constants outsidesrc/core/client.ts
Files:
src/registries/npm.tssrc/registries/pypi.tssrc/index.tssrc/registries/rubygems.tssrc/core/types.tssrc/helpers.tssrc/registries/packagist.tssrc/registries/cargo.ts
src/registries/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/registries/**/*.ts: Do not parse PURLs outsidesrc/core/purl.ts; callcreateFromPURL/parsePURL
Do not bypassClientfor direct fetch logic in registries
Files:
src/registries/npm.tssrc/registries/pypi.tssrc/registries/rubygems.tssrc/registries/packagist.tssrc/registries/cargo.ts
src/**/index.ts
📄 CodeRabbit inference engine (src/AGENTS.md)
Keep barrels (
src/index.ts,src/core/index.ts,src/cache/index.ts) as canonical export surfaces
Files:
src/index.ts
src/index.ts
📄 CodeRabbit inference engine (src/AGENTS.md)
Extend public exports through
src/index.tsand keep grouping order stable (Core, Errors, Helpers, Types, Cache)Public API is export-barrel-driven; avoid hidden side-channel exports
Files:
src/index.ts
src/core/types.ts
📄 CodeRabbit inference engine (src/AGENTS.md)
Changes to shared models in
src/core/types.tsimpact all registries and helpers and should be carefully considered
Files:
src/core/types.ts
src/core/**/*.ts
📄 CodeRabbit inference engine (src/core/AGENTS.md)
src/core/**/*.ts: Throw typed errors (InvalidPURLError,NotFoundError,RateLimitError) instead of plainErrorin core flows
VersionStatusandScopeare closed unions; keep adapter outputs inside allowed values
No duplicate PURL parsing utilities outsidepurl.ts- usesrc/core/purl.tsas the single source of truth for PURL validation and normalization
Files:
src/core/types.ts
src/helpers.ts
📄 CodeRabbit inference engine (src/AGENTS.md)
Add convenience API to
src/helpers.tsand wrapcreateFromPURL; preserve normalization path
Files:
src/helpers.ts
test/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Test files must use
*.test.tsnaming convention
Files:
test/unit/helpers.test.tstest/unit/cached-registry.test.tstest/unit/registry.test.ts
test/unit/**/*.test.ts
📄 CodeRabbit inference engine (test/AGENTS.md)
Unit tests must be placed in
test/unit/directory and use mocks/spies rather than real HTTP
Files:
test/unit/helpers.test.tstest/unit/cached-registry.test.tstest/unit/registry.test.ts
test/{unit,e2e}/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Use Vitest globals with
test/unitandtest/e2esplit
Files:
test/unit/helpers.test.tstest/unit/cached-registry.test.tstest/unit/registry.test.ts
🧠 Learnings (3)
📚 Learning: 2026-03-05T16:31:28.455Z
Learnt from: CR
Repo: oritwoen/regxa PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T16:31:28.455Z
Learning: Applies to src/index.ts : Public API is export-barrel-driven; avoid hidden side-channel exports
Applied to files:
src/index.ts
📚 Learning: 2026-03-05T16:31:28.455Z
Learnt from: CR
Repo: oritwoen/regxa PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T16:31:28.455Z
Learning: Applies to src/registries/*.ts : Implement `Registry` interface and register via side-effect import for new ecosystem support
Applied to files:
test/unit/cached-registry.test.tstest/unit/registry.test.ts
📚 Learning: 2026-03-05T16:31:28.455Z
Learnt from: CR
Repo: oritwoen/regxa PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-03-05T16:31:28.455Z
Learning: Applies to src/registries/**/*.ts : Do not parse PURLs outside `src/core/purl.ts`; call `createFromPURL`/`parsePURL`
Applied to files:
test/unit/registry.test.ts
🔇 Additional comments (8)
src/core/types.ts (1)
53-53: Good interface extension for README URL support.This keeps URL resolution concerns inside adapters and maintains a consistent
URLBuildercontract.src/registries/pypi.ts (1)
223-228: PyPI README URL mapping looks correct.Using the normalized project URL (optionally version-specific) matches the intended ecosystem behavior.
src/registries/packagist.ts (1)
251-254: Packagist README URL implementation is aligned with the adapter contract.Returning the package page here is consistent with the documented registry behavior.
src/registries/rubygems.ts (1)
221-224: RubyGems README URL builder is consistent with expected behavior.The versioned/unversioned page mapping is coherent with the registry URL strategy.
src/registries/npm.ts (1)
328-331: npm README URL generation looks good.The jsDelivr path and optional version suffix are implemented correctly for the intended behavior.
src/index.ts (1)
27-27: Public API export update is correctly wired through the barrel.Adding
resolveReadmeUrlhere keeps the new helper discoverable via the canonical entrypoint.
Based on learnings: Applies to src/index.ts : Public API is export-barrel-driven; avoid hidden side-channel exports.test/unit/helpers.test.ts (1)
2-2: Good coverage for the new README URL helper path.The added
resolveReadmeUrltests validate version propagation, default behavior, and name wiring clearly, and the updated stub aligns with the expandedURLBuildercontract.Also applies to: 35-35, 140-155
test/unit/registry.test.ts (1)
29-29: Consistent mock contract update across registry tests.Nice job updating every mock
urls()builder to includereadme, which keeps these tests aligned with the expandedURLBuilderinterface.Also applies to: 59-59, 92-92, 140-140, 171-171, 204-204, 245-245, 279-279, 313-313
Closes #5
Every registry handles README URLs differently, so this adds
readme(name, version?)to URLBuilder:cdn.jsdelivr.net/npm/{name}@{version}/README.md)/api/v1/crates/{name}/{version}/readme)Also adds
resolveReadmeUrl(pkg, urls, version?)to pick the right URL given a Package and its URLBuilder.