Conversation
buildPURL took 6 positional args which got unreadable fast, and it couldn't encode qualifiers or subpath at all. Switched to an object param that matches ParsedPURL shape, so parsePURL output feeds directly back into buildPURL for round-trips. Co-Authored-By: Aei <256851514+aeitwoen@users.noreply.github.com>
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad727c08bd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
1 issue found across 2 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="src/core/purl.ts">
<violation number="1" location="src/core/purl.ts:127">
P2: Subpath encoding escapes `/`, but PURL subpath requires `/` separators to remain unescaped. Encode each segment instead so canonical subpaths like `a/b` remain `a/b` rather than `a%2Fb`.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
encodeURIComponent on the whole subpath was escaping / into %2F, breaking the PURL spec which requires / as literal segment separators. Now both parse and build handle subpath per-segment.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/core/purl.ts`:
- Around line 113-127: The namespace and subpath are currently encoded as whole
strings which encodes '/' characters; update the logic that builds the purl (the
code using parts.namespace, parts.subpath, encodeURIComponent and purl) to split
namespace and subpath on '/' into segments, percent-encode each segment with
encodeURIComponent, then rejoin with '/' so the separators remain unencoded; for
namespace do: parts.namespace.split('/').map(encodeURIComponent).join('/') and
append the trailing '/', and for subpath do:
parts.subpath.split('/').map(encodeURIComponent).join('/') after the '#' so the
resulting PURL matches ECMA-427 segment-wise encoding.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 81dbd0a4-f337-4868-8544-0b57ec9201d3
📒 Files selected for processing (2)
src/core/purl.tstest/unit/purl.test.ts
📜 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)
- GitHub Check: cubic · AI code reviewer
🔇 Additional comments (1)
test/unit/purl.test.ts (1)
159-235: Good test migration for the object-based API.This suite now covers the updated call shape plus qualifiers, subpath, and parse→build round-trip behavior well.
Same issue as subpath — multi-segment namespaces had / escaped to %2F. Per ECMA-427, segment separators stay literal.
Replaces the 6 positional args on buildPURL with a single object that matches ParsedPURL. This means parsePURL output goes straight into buildPURL for lossless round-trips, qualifiers and subpath included.
Closes #1
Summary by cubic
Switched buildPURL to a single object parameter that matches ParsedPURL, with support for qualifiers and subpath. Subpaths and namespaces are now encoded per segment so "/" remains literal, enabling spec-compliant, lossless parse→build; stringifyPURL is removed.
Written for commit c411f34. Summary will update on new commits.