Conversation
Surface documentation URLs from all ecosystems as a first-class Package field. Add selectVersion() for best-version resolution and resolveDocsUrl() for fallback-chain docs URL lookup — both needed for ecosystem-agnostic consumers like skilld.
|
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 (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used📓 Path-based instructions (9)test/**/*.test.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
test/unit/**/*.test.ts📄 CodeRabbit inference engine (test/AGENTS.md)
Files:
src/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/!(purl).ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/**/!(client).ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/helpers.ts📄 CodeRabbit inference engine (src/AGENTS.md)
Files:
src/registries/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/{cache,registries}/**/*.ts📄 CodeRabbit inference engine (AGENTS.md)
Files:
src/registries/*.ts📄 CodeRabbit inference engine (src/AGENTS.md)
Files:
🔇 Additional comments (8)
📝 WalkthroughWalkthroughAdds a required Changes
Sequence Diagram(s)sequenceDiagram
participant Consumer as Consumer
participant Registry as Registry.fetchPackage
participant Helpers as Helpers.selectVersion / resolveDocsUrl
participant URLBuilder as URLBuilder
Consumer->>Registry: request package + versions
Registry-->>Consumer: returns Package (includes documentation) + Versions
Consumer->>Helpers: selectVersion(versions, {requested?, latest?})
Helpers-->>Consumer: Version | null
Consumer->>Helpers: resolveDocsUrl(package, URLBuilder, version?)
Helpers->>URLBuilder: documentation url request (if needed)
URLBuilder-->>Helpers: documentation URL
Helpers-->>Consumer: docs URL (package.documentation or homepage or URLBuilder)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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 |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 11 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="test/unit/helpers.test.ts">
<violation number="1" location="test/unit/helpers.test.ts:91">
P3: Test description is incorrect: it claims null is returned, but the assertion expects fallback to the latest version.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/registries/rubygems.ts (1)
17-25:⚠️ Potential issue | 🟠 MajorAdd
documentation_uritoRubyGemsGemResponseinterface and read from the top-level field.Line 86 reads documentation from
data.metadata?.documentation_uriwith an unsafe cast, but according to the official RubyGems API,documentation_uriis a top-level field. The current implementation reads from the wrong location and will always produce empty documentation URLs.Update the interface to include
documentation_uri?: stringand use type-safe narrowing:Proposed fix
interface RubyGemsGemResponse { name: string version?: string description: string homepage_uri?: string + documentation_uri?: string source_code_uri?: string licenses?: string[] metadata?: Record<string, unknown> @@ return { name: data.name, description: data.description || '', homepage: data.homepage_uri || '', - documentation: (data.metadata?.documentation_uri as string) || '', + documentation: typeof data.documentation_uri === 'string' + ? data.documentation_uri + : typeof data.metadata?.['documentation_uri'] === 'string' + ? data.metadata['documentation_uri'] + : '', repository, licenses,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/registries/rubygems.ts` around lines 17 - 25, The RubyGems response interface is missing documentation_uri and the code currently reads documentation from data.metadata?.documentation_uri (unsafe cast); update the RubyGemsGemResponse interface to include documentation_uri?: string and change the mapping logic that builds the package documentation URL to read from data.documentation_uri (use type-safe checks, e.g., typeof data.documentation_uri === "string" or truthiness) instead of data.metadata, ensuring you remove the unsafe cast and fall back to other top-level fields like homepage_uri/source_code_uri only if documentation_uri is absent; reference the RubyGemsGemResponse interface and the code that accesses data.metadata?.documentation_uri to locate the change.
🧹 Nitpick comments (1)
test/unit/helpers.test.ts (1)
91-95: Rename this test title to match actual behavior.The case currently states "returns null" but asserts fallback to
latest. The assertion looks right; the title is misleading.Suggested title-only fix
- it('returns null when requested version does not exist', () => { + it('falls back to latest when requested version does not exist', () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/unit/helpers.test.ts` around lines 91 - 95, The test title is misleading: update the it() description for the test that calls selectVersion(versions, { requested: '9.9.9', latest: '2.0.0' }) so it reflects that the function falls back to the latest version rather than returning null (e.g., change "returns null when requested version does not exist" to "returns latest when requested version does not exist"); keep the existing assertion that expects result?.number toBe '2.0.0' and only change the test description string in the unit test surrounding selectVersion.
🤖 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/helpers.ts`:
- Around line 70-87: The fallback in selectVersion currently returns the first
Version with status === '' which can be stale; update selectVersion so that when
neither requested nor latest matches it chooses the newest clean version from
the versions array (e.g., filter by v.status === '' then pick the max by
semantic version order), preserving the existing requested/latest checks; refer
to selectVersion, versions, requested, latest and ensure the comparison uses a
proper semver-aware comparison rather than input order.
In `@test/unit/helpers.test.ts`:
- Line 1: Remove the unnecessary direct imports of Vitest globals by deleting
the import statement that lists describe, it, expect (and any occurrences of vi,
beforeEach, afterEach) at the top of the test files and rely on the configured
globals instead; update tests such as the one referencing describe/it/expect in
helpers.test.ts (and the other unit tests mentioned) so they use the global
functions without importing them.
---
Outside diff comments:
In `@src/registries/rubygems.ts`:
- Around line 17-25: The RubyGems response interface is missing
documentation_uri and the code currently reads documentation from
data.metadata?.documentation_uri (unsafe cast); update the RubyGemsGemResponse
interface to include documentation_uri?: string and change the mapping logic
that builds the package documentation URL to read from data.documentation_uri
(use type-safe checks, e.g., typeof data.documentation_uri === "string" or
truthiness) instead of data.metadata, ensuring you remove the unsafe cast and
fall back to other top-level fields like homepage_uri/source_code_uri only if
documentation_uri is absent; reference the RubyGemsGemResponse interface and the
code that accesses data.metadata?.documentation_uri to locate the change.
---
Nitpick comments:
In `@test/unit/helpers.test.ts`:
- Around line 91-95: The test title is misleading: update the it() description
for the test that calls selectVersion(versions, { requested: '9.9.9', latest:
'2.0.0' }) so it reflects that the function falls back to the latest version
rather than returning null (e.g., change "returns null when requested version
does not exist" to "returns latest when requested version does not exist"); keep
the existing assertion that expects result?.number toBe '2.0.0' and only change
the test description string in the unit test surrounding selectVersion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7d4cc3b4-4282-43d7-abc4-5884a7b4c2e9
📒 Files selected for processing (11)
README.mdsrc/core/types.tssrc/helpers.tssrc/index.tssrc/registries/cargo.tssrc/registries/npm.tssrc/registries/packagist.tssrc/registries/pypi.tssrc/registries/rubygems.tstest/unit/helpers.test.tstest/unit/registries.test.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (13)
src/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Keep TypeScript imports with
.tsextension style in ESM-only source files
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
Files:
src/index.tssrc/core/types.tssrc/registries/pypi.tssrc/registries/rubygems.tssrc/helpers.tssrc/registries/packagist.tssrc/registries/npm.tssrc/registries/cargo.ts
src/**/!(purl).ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not parse PURLs outside
src/core/purl.ts; callcreateFromPURLorparsePURLinstead
Files:
src/index.tssrc/core/types.tssrc/registries/pypi.tssrc/registries/rubygems.tssrc/helpers.tssrc/registries/packagist.tssrc/registries/npm.tssrc/registries/cargo.ts
src/**/!(client).ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not duplicate retry and backoff constants outside
src/core/client.ts; centralize retry configuration in theClientclass
Files:
src/index.tssrc/core/types.tssrc/registries/pypi.tssrc/registries/rubygems.tssrc/helpers.tssrc/registries/packagist.tssrc/registries/npm.tssrc/registries/cargo.ts
src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Public API is export-barrel-driven; avoid hidden side-channel exports and export through
src/index.tsExtend public exports through
src/index.tsand keep grouping order stable (Core, Errors, Helpers, Types, Cache)
Files:
src/index.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/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
test/**/*.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Tests must use Vitest globals with
test/unitfor unit tests andtest/e2efor integration/smoke testsTest files must use
*.test.tsnaming convention
Files:
test/unit/helpers.test.tstest/unit/registries.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/registries.test.ts
src/registries/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/registries/**/*.ts: Do not bypassClientfor direct fetch logic in registries; use theClientclass fromsrc/core/client.ts
Registries are plugin-like via registration factories; do not use hardcoded switch logic
Side-effect ecosystem registration is intentional; import registries via side-effect imports likeimport 'unpux/registries'
Files:
src/registries/pypi.tssrc/registries/rubygems.tssrc/registries/packagist.tssrc/registries/npm.tssrc/registries/cargo.ts
src/{cache,registries}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not hardcode cache TTL in random modules; use
DEFAULT_TTLfromsrc/cache/lockfile.ts
Files:
src/registries/pypi.tssrc/registries/rubygems.tssrc/registries/packagist.tssrc/registries/npm.tssrc/registries/cargo.ts
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 types
Files:
src/registries/pypi.tssrc/registries/rubygems.tssrc/registries/packagist.tssrc/registries/npm.tssrc/registries/cargo.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
🔇 Additional comments (10)
src/core/types.ts (1)
8-8:Packagemodel extension is consistent.Line 8 cleanly extends the normalized package contract with
documentation, and it aligns with the helper-level docs fallback behavior.README.md (1)
218-218: README type snippet stays in sync with the code model.Line 218 correctly documents the new
documentation: stringfield inPackage.src/registries/packagist.ts (1)
92-92: Adapter contract alignment looks good.Line 92 ensures the Packagist adapter emits the required
documentationfield in the normalizedPackageshape.src/registries/npm.ts (1)
106-106: NPM package normalization remains consistent.Line 106 updates the adapter output to satisfy the required
Package.documentationcontract.src/registries/pypi.ts (1)
89-89: Good source-to-model mapping for docs URL.Line 89 correctly exposes PyPI
project_urls['Documentation']through normalizedPackage.documentation.src/registries/cargo.ts (1)
133-147: Cargo adapter enrichment looks solid.The added documentation field and metadata payloads are well-contained and keep the normalized
Package/Versioncontracts intact.Also applies to: 175-179
src/helpers.ts (1)
97-99: Docs URL fallback chain is clean and practical.
resolveDocsUrlat Lines 97-99 applies the intended precedence without duplicating registry-specific logic.src/index.ts (1)
25-26: Public helper exports are correctly surfaced through the barrel.This keeps the API discoverable and consistent with the existing export structure.
test/unit/helpers.test.ts (1)
39-128: Helper test coverage is thorough and well-targeted.The scenarios around status filtering, fallback order, and docs URL precedence are solid for regression protection.
test/unit/registries.test.ts (1)
182-187: These added assertions correctly lock in the new normalization contract.Good additions for guarding documentation mapping and cargo metadata extraction at both package and version levels.
Also applies to: 285-288, 354-354
Sort selectVersion fallback by publishedAt newest-first instead of input order. Read RubyGems documentation_uri from top-level field. Remove redundant Vitest imports. Fix misleading test description.
aeitwoen
changed the title
2 participants
Closes #3
Adds a
documentationfield toPackage- populated from Cargo, PyPI, and RubyGems (npm and Packagist don't expose one). Also addsselectVersion()for picking the best version andresolveDocsUrl()for the docs URL fallback chain.Summary by cubic
Adds a first-class Package.documentation field and helpers to pick a usable version and resolve docs URLs across ecosystems. Implements #3 and improves version fallback and RubyGems docs extraction.
Written for commit 76b8f6a. Summary will update on new commits.