Repository navigation
Render unusual supply-chain signals in dependency details - #29
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (2)
📝 WalkthroughWalkthroughUpdates package metadata; adds supply-chain signal indexing and rendering to the report UI; tightens CLI parsing with a new Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Builder as Builder (scripts/build-report.ts)
participant Assets as Report Assets (report.iife.js)
participant Browser as Report UI (report-ui/main.ts)
participant User as User (browser)
Builder->>Builder: Read minified report.iife.js
Builder->>Builder: Sanitize ambiguous tokens (e.g., "?.<digit>" → "? .<digit>")
Builder->>Assets: Write sanitized report.iife.js (embedded JS_CONTENT)
Browser->>Assets: Load report assets (sanitized JS)
Browser->>Browser: init()\nbuildSupplyChainSignalIndex()
User->>Browser: Open dependency detail
Browser->>Browser: Lookup dependency key in index\nAttach supplyChainSignals to detail render
Browser-->>User: Render dependency detail with "Supply chain source"
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 4/5 reviews remaining, refill in 12 minutes. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@report-ui/main.ts`:
- Around line 953-982: buildSupplyChainSignalIndex can add the same
SupplyChainSignal multiple times when both candidate keys resolve to the same
dep; update the loop that iterates candidates in buildSupplyChainSignalIndex so
that after a successful resolve via resolveDepLinkTarget you call add(depKey,
signal) once and break out of the candidate loop (or alternatively dedupe
resolved depKey values before calling add), ensuring you still fall back to
resolveDepKeyByNameFromSet only if no candidate matched; reference the inner
variables candidates, depKey, matched, the add helper, and the functions
resolveDepLinkTarget / resolveDepKeyByNameFromSet when making the change.
In `@src/cli.ts`:
- Around line 1229-1236: takeOptionValue currently rejects any value beginning
with "-" which breaks path-valued options; change takeOptionValue(args:
string[], option: string, allowLeadingDash = false) to only enforce the
startsWith("-") check when allowLeadingDash is false, and return the shifted
value otherwise, and update any call sites that parse filesystem paths to call
takeOptionValue(..., true) (leave existing flag callers unchanged).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3840b101-5cf4-4287-9a4e-d4d0d0d949fe
⛔ Files ignored due to path filters (4)
dist/cli.jsis excluded by!**/dist/**dist/report-assets.jsis excluded by!**/dist/**dist/report.jsis excluded by!**/dist/**report-ui/dist/report.iife.jsis excluded by!**/dist/**
📒 Files selected for processing (7)
package.jsonreport-ui/main.tsreport-ui/vite.config.tsscripts/build-report.tssrc/cli.test.tssrc/cli.tssrc/report-assets.ts
💤 Files with no reviewable changes (1)
- report-ui/vite.config.ts
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @JosephMaynard. The following files were modified: * `report-ui/main.ts` * `scripts/build-report.ts` * `src/cli.ts` These files were ignored: * `src/cli.test.ts` These file types are not supported: * `README.md` * `package.json`
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 `@report-ui/main.ts`:
- Around line 997-1015: The current loop uses resolveDepLinkTarget which may
fall back to name-only matches and thus attach exact-version signals to the
wrong package; implement and use a strict resolver (e.g.,
resolveExactDepLinkTarget as described) that only returns an exact depKey or npm
alias and does not perform name-only fallbacks, call that resolver when
iterating candidates (replace resolveDepLinkTarget(candidate, linkableKeys,
keysByName) with resolveExactDepLinkTarget(candidate, linkableKeys)) and keep
the existing fallback to resolveDepKeyByNameFromSet(signal.packageName,
linkableKeys, keysByName) only for the unmatched case so exact-match phases
remain exact.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 941d69d4-ce21-4b95-84af-26a4bd26cdb3
📒 Files selected for processing (3)
report-ui/main.tsscripts/build-report.tssrc/cli.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/build-report.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 5 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 5 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
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 `@report-ui/main.ts`:
- Around line 1050-1058: The JSDoc block above the renderRootPackageList
function is missing its closing "*/", which causes the function declaration to
be commented out; fix this by adding the terminating "*/" immediately before the
"function renderRootPackageList(" line so the JSDoc is properly closed and
renderRootPackageList (and its params like packages, maxShow, linkableKeys,
keysByName) are parsed as code.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 65a5c908-a06e-444c-bd86-0c09934bce85
⛔ Files ignored due to path filters (3)
dist/cli.jsis excluded by!**/dist/**dist/report-assets.jsis excluded by!**/dist/**report-ui/dist/report.iife.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
report-ui/main.tssrc/report-assets.ts
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 4 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 4 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
♻️ Duplicate comments (1)
report-ui/main.ts (1)
1041-1043:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep versioned signals out of the name-only fallback.
If
signal.packageVersionis present but the exactname@versioncandidate misses, this branch can still pin the signal to a different dependency with the same name. That puts a version-specific warning on the wrong card. Restrict this fallback to versionless signals or leave the signal unassigned.Suggested fix
- if (!matched && signal.packageName) { + if (!matched && signal.packageName && !signal.packageVersion) { const depKey = resolveDepKeyByNameFromSet(signal.packageName, linkableKeys, keysByName); if (depKey) add(depKey, signal); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@report-ui/main.ts` around lines 1041 - 1043, The current fallback assigns a signal by name even when signal.packageVersion exists, which can pin a versioned signal to the wrong dependency; in the block using resolveDepKeyByNameFromSet and add, restrict the fallback to only versionless signals by changing the condition to require that signal.packageVersion is falsy (e.g., if (!matched && signal.packageName && !signal.packageVersion) { ... }), so resolveDepKeyByNameFromSet(linkableKeys, keysByName) is only used for name-only signals and versioned signals remain unassigned when their exact name@version candidate misses.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@report-ui/main.ts`:
- Around line 1041-1043: The current fallback assigns a signal by name even when
signal.packageVersion exists, which can pin a versioned signal to the wrong
dependency; in the block using resolveDepKeyByNameFromSet and add, restrict the
fallback to only versionless signals by changing the condition to require that
signal.packageVersion is falsy (e.g., if (!matched && signal.packageName &&
!signal.packageVersion) { ... }), so resolveDepKeyByNameFromSet(linkableKeys,
keysByName) is only used for name-only signals and versioned signals remain
unassigned when their exact name@version candidate misses.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ffd4ca5-7799-414b-8808-6de3a65b5ffe
⛔ Files ignored due to path filters (3)
dist/cli.jsis excluded by!**/dist/**dist/report-assets.jsis excluded by!**/dist/**report-ui/dist/report.iife.jsis excluded by!**/dist/**
📒 Files selected for processing (2)
report-ui/main.tssrc/report-assets.ts
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores
Documentation