Chore/sp 4217 enhance component entity output#8
Merged
agustingroh merged 2 commits intomainfrom Apr 1, 2026
Merged
Conversation
…lds to component entity
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 9 minutes and 49 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds a semver resolution utility Changes
Sequence Diagram(s)sequenceDiagram
participant Worker as componentVersionWorker
participant Resolver as componentResolver
participant Service as ComponentService
Worker->>Resolver: GetComponent(ctx, purl)
alt component found
Resolver->>Worker: Component (metadata, maybe Version)
Worker->>Resolver: GetComponentVersions(ctx, purl)
Resolver->>Service: fetch versions for purl
Service-->>Resolver: versions list
Resolver-->>Worker: ComponentVersionsResponse (versions)
Worker->>Worker: populate processedComponent.Versions
else not found / version missing
Resolver-->>Worker: ErrComponentNotFound / ErrVersionNotFound
Worker->>Worker: set processedComponent.Status / StatusCode
end
Worker->>Worker: enqueue / persist processedComponent
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
agustingroh
force-pushed
the
chore/SP-4217-enhance-component-entity-output
branch
from
ee9e219 to
a263b96
Compare
scanoss-qg
approved these changes
Mar 31, 2026
scanoss-qg
left a comment
scanoss-qg
left a comment
There was a problem hiding this comment.
LGTM considering new changes on model helper
agustingroh
force-pushed
the
chore/SP-4217-enhance-component-entity-output
branch
from
a263b96 to
46ee7c1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
componenthelper/component_worker.go (2)
94-97:⚠️ Potential issue | 🟠 MajorSuccessful resolutions still leave
Versionsempty.The new public field is only populated in the error branch, so the common success path never returns the available version list even after adding
Component.Versions. If this field is part of the enhanced component output, populate it here as well or document it as error-only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@componenthelper/component_worker.go` around lines 94 - 97, The success path never populates the new public Versions field, so set processedComponent.Versions = component.Versions before sending the result; specifically, in the block around component.Version != "" and just before results <- processedComponent, assign the component's Versions slice/map into processedComponent.Versions so successful responses include the available version list (mirroring what the error branch does).
72-88:⚠️ Potential issue | 🟠 MajorOnly call
GetComponentVersionsforErrVersionNotFound.This block currently runs for every non-
ErrComponentNotFoundfailure, so transient/backend errors now trigger a second lookup and still get flattened toComponentNotFound. It also logserrinstead oferrCompVersion, which hides the actual versions-lookup failure.Suggested change
- // Add list of component versions - processedComponent.Versions = []string{} - componentVersions, errCompVersion := resolver.GetComponentVersions(ctx, j.Purl) - if errCompVersion != nil { - s.Errorf("Failed to get component versions: %s, %v", j.Purl, err) - } else { - processedComponent.Versions = componentVersions.Versions - } - if errors.Is(err, services.ErrVersionNotFound) { + processedComponent.Versions = []string{} + componentVersions, errCompVersion := resolver.GetComponentVersions(ctx, j.Purl) + if errCompVersion != nil { + s.Errorf("Failed to get component versions: %s, %v", j.Purl, errCompVersion) + } else { + processedComponent.Versions = componentVersions.Versions + } s.Warnf("Failed to get component version: %s, %s", j.Purl, j.Requirement) processedComponent.Status.StatusCode = domain.VersionNotFound processedComponent.Status.Message = "Component version not found" results <- processedComponent continue🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@componenthelper/component_worker.go` around lines 72 - 88, The code currently calls resolver.GetComponentVersions() for every non-ErrComponentNotFound path and logs the wrong error variable; instead, only invoke resolver.GetComponentVersions(ctx, j.Purl) when errors.Is(err, services.ErrVersionNotFound) and use errCompVersion in any log messages; change the control flow in the block around processedComponent.Versions so that: 1) if errors.Is(err, services.ErrVersionNotFound) then call resolver.GetComponentVersions and on errCompVersion log s.Errorf with errCompVersion (not err), set processedComponent.Status.StatusCode = domain.VersionNotFound and Message accordingly and send results <- processedComponent; 2) for other errors (including transient/backend) do not call GetComponentVersions and keep the existing s.Errorf("Failed to get component: ...", err) handling. Ensure you reference resolver.GetComponentVersions, errCompVersion, errors.Is(err, services.ErrVersionNotFound), and processedComponent when making the change.
🧹 Nitpick comments (1)
componenthelper/component_worker_test.go (1)
48-53: Make unexpectedGetComponentVersionscalls fail fast in the mock.Returning
types.ComponentVersionsResponse{}, nilwhen the hook is unset lets worker changes call the new resolver method without any test noticing. Returning an error here keeps the mock honest and makes those regressions visible.Suggested change
func (m *mockComponentResolver) GetComponentVersions(ctx context.Context, purl string) (types.ComponentVersionsResponse, error) { if m.getComponentVersionsFn != nil { return m.getComponentVersionsFn(ctx, purl) } - return types.ComponentVersionsResponse{}, nil + return types.ComponentVersionsResponse{}, errors.New("unexpected GetComponentVersions call") }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@componenthelper/component_worker_test.go` around lines 48 - 53, The mock's GetComponentVersions currently returns an empty success response when getComponentVersionsFn is nil, hiding unexpected calls; update mockComponentResolver.GetComponentVersions to return a non-nil error (e.g., fmt.Errorf("unexpected GetComponentVersions call")) when getComponentVersionsFn == nil so tests fail fast and reveal unintended usage instead of silently succeeding; reference the getComponentVersionsFn hook in your change and ensure callers in tests that expect this behavior are updated to set the hook or assert the error.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 10-19: The changelog incorrectly marks "## [0.6.0] - 2026-04-01"
as released; keep the 0.6.0 notes under the "## [Unreleased]" section until the
tag is actually created: move the entire 0.6.0 block (the header and its
"Added"/"Changed" bullet points, including the compare link if present) back
under "## [Unreleased]" and remove or defer any release date/compare link for
"0.6.0" so the file only shows the Unreleased entry until the release/tag is
cut.
In `@componenthelper/utils/semver_test.go`:
- Around line 54-63: The test incorrectly expects "~2.0.0" to match 2.1.0;
update the semver handling to stop stripping constraint operators and instead
evaluate candidates against the actual constraint (use Masterminds/semver
Constraint parsing and .Check/version satisfies logic) and only fall back to
nearest-version heuristics if no candidate satisfies the parsed constraint;
update the test case named "with ~ operator" so its expected result reflects
that no candidate satisfies "~2.0.0" (or adjust to a valid satisfying
candidate), and ensure the resolver function that previously removed operators
now uses constraint parsing to filter candidates first.
In `@componenthelper/utils/semver.go`:
- Line 35: The operator stripping misses ^ and = and can leave a leading space
(e.g., ">= 1.2.3"), causing semver.Parse to fail; update the operatorRegex used
by stripOperators to include ^ and = and optionally consume any following space
(so it matches operators like "^", "=", ">=", "<=", "~", ">", "<" with optional
whitespace), and ensure stripOperators trims remaining whitespace (use
strings.TrimSpace before/after replacement) so the returned version string is
normalized before calling semver.Parse; references: operatorRegex and
stripOperators (the semver.Parse caller should receive the trimmed,
operator-stripped string).
- Around line 60-71: The loop that parses candidates treats invalid semver
strings as zeroVersion (v0.0.0) and still sets nearestOriginal to the raw
(invalid) string; change the logic in the candidates loop (the block using
semver.NewVersion, zeroVersion, semverDistance, minDistance, nearest, and
nearestOriginal) so that invalid parses are skipped (do not consider
v==zeroVersion from a failed parse) or only update nearestOriginal when
semver.NewVersion succeeded; ensure only successfully parsed semver values can
win nearest/minDistance comparisons and that nearestOriginal stores the original
string only for valid parsed versions.
---
Outside diff comments:
In `@componenthelper/component_worker.go`:
- Around line 94-97: The success path never populates the new public Versions
field, so set processedComponent.Versions = component.Versions before sending
the result; specifically, in the block around component.Version != "" and just
before results <- processedComponent, assign the component's Versions slice/map
into processedComponent.Versions so successful responses include the available
version list (mirroring what the error branch does).
- Around line 72-88: The code currently calls resolver.GetComponentVersions()
for every non-ErrComponentNotFound path and logs the wrong error variable;
instead, only invoke resolver.GetComponentVersions(ctx, j.Purl) when
errors.Is(err, services.ErrVersionNotFound) and use errCompVersion in any log
messages; change the control flow in the block around
processedComponent.Versions so that: 1) if errors.Is(err,
services.ErrVersionNotFound) then call resolver.GetComponentVersions and on
errCompVersion log s.Errorf with errCompVersion (not err), set
processedComponent.Status.StatusCode = domain.VersionNotFound and Message
accordingly and send results <- processedComponent; 2) for other errors
(including transient/backend) do not call GetComponentVersions and keep the
existing s.Errorf("Failed to get component: ...", err) handling. Ensure you
reference resolver.GetComponentVersions, errCompVersion, errors.Is(err,
services.ErrVersionNotFound), and processedComponent when making the change.
---
Nitpick comments:
In `@componenthelper/component_worker_test.go`:
- Around line 48-53: The mock's GetComponentVersions currently returns an empty
success response when getComponentVersionsFn is nil, hiding unexpected calls;
update mockComponentResolver.GetComponentVersions to return a non-nil error
(e.g., fmt.Errorf("unexpected GetComponentVersions call")) when
getComponentVersionsFn == nil so tests fail fast and reveal unintended usage
instead of silently succeeding; reference the getComponentVersionsFn hook in
your change and ensure callers in tests that expect this behavior are updated to
set the hook or assert the error.
🪄 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: e33cc7f5-52b7-4e84-ac6d-b76aeffccf57
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (10)
CHANGELOG.mdREADME.mdcomponenthelper/component_entity.gocomponenthelper/component_sanitise.gocomponenthelper/component_sanitise_test.gocomponenthelper/component_worker.gocomponenthelper/component_worker_test.gocomponenthelper/utils/semver.gocomponenthelper/utils/semver_test.gogo.mod
There was a problem hiding this comment.
🧹 Nitpick comments (1)
componenthelper/component_worker.go (1)
84-91: Normalize resolver version-list output to avoidnilslice regressions.Line 90 can replace the pre-initialized empty slice with
nilwhen the resolver returns no versions. Keeping[]consistently can make downstream JSON/output handling more predictable.Proposed adjustment
- processedComponent.Versions = componentVersions.Versions + if componentVersions.Versions == nil { + processedComponent.Versions = []string{} + } else { + processedComponent.Versions = componentVersions.Versions + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@componenthelper/component_worker.go` around lines 84 - 91, The resolver may return an empty version list which should be normalized to nil to avoid downstream JSON/slice regressions: after calling resolver.GetComponentVersions(ctx, j.Purl) in the block that assigns processedComponent.Versions, check componentVersions.Versions and if its length is zero set processedComponent.Versions = nil (otherwise assign the returned slice); update the assignment around the resolver.GetComponentVersions / processedComponent.Versions logic to ensure empty lists become nil.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@componenthelper/component_worker.go`:
- Around line 84-91: The resolver may return an empty version list which should
be normalized to nil to avoid downstream JSON/slice regressions: after calling
resolver.GetComponentVersions(ctx, j.Purl) in the block that assigns
processedComponent.Versions, check componentVersions.Versions and if its length
is zero set processedComponent.Versions = nil (otherwise assign the returned
slice); update the assignment around the resolver.GetComponentVersions /
processedComponent.Versions logic to ensure empty lists become nil.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a03d38f2-4b31-4fd8-ac59-666bc77fcd76
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (8)
CHANGELOG.mdREADME.mdcomponenthelper/component_entity.gocomponenthelper/component_worker.gocomponenthelper/component_worker_test.gocomponenthelper/utils/semver.gocomponenthelper/utils/semver_test.gogo.mod
✅ Files skipped from review due to trivial changes (3)
- go.mod
- componenthelper/utils/semver_test.go
- CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
…d in Component entity
agustingroh
force-pushed
the
chore/SP-4217-enhance-component-entity-output
branch
from
46ee7c1 to
9fedcaf
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores