chore(cryptography):SP-4324 replace error_message/error_code witn inf…#76
Conversation
|
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 39 minutes and 2 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 (3)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughA breaking change replaces per-component Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
protobuf/scanoss/api/cryptography/v2/README.md (1)
405-421:⚠️ Potential issue | 🟡 MinorStray blank line inside the JSON example.
Line 414 is an empty line between
"info_code": "NO_INFO"and the closing}of thecomponentobject. This renders fine in Markdown but is unusual in a copy-paste-able JSON sample and inconsistent with the other examples in the file (e.g., the sample at lines 48–63). Minor:"hints": [], "info_message": "No info found for: pkg:github/example/simple-utility", "info_code": "NO_INFO" - },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protobuf/scanoss/api/cryptography/v2/README.md` around lines 405 - 421, The JSON example under "Component with No Cryptographic Hints" contains a stray blank line between the "info_code": "NO_INFO" entry and the closing brace of the "component" object; remove that empty line so the component object closes immediately after "info_code" (keep the keys "purl", "version", "hints", "info_message", "info_code" intact) to make the snippet a clean, copy-paste-able JSON example consistent with other samples.protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json (1)
882-892:⚠️ Potential issue | 🟡 MinorThe added
description:insidejson_schemaoverwrites the original message description in generated OpenAPI — inconsistent and a doc regression.In
protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.proto(line 447 forComponentAlgorithmsResponseand line 927 forComponentHintsInRangeResponse), the newdescription:insideopenapiv2_schema.json_schemareplaces the class-level description generated from the proto comment. The effect is visible here:
- Line 892
v2ComponentAlgorithmsResponse.descriptionis now only the "Success example. For error cases…" text. The original"Response message for GetComponentAlgorithms method. Contains cryptographic algorithm information for a single software component including algorithm names, strength classifications, and analysis metadata."is gone.- Line 999
v2ComponentHintsInRangeResponse.descriptionshows the same issue.For comparison,
v2ComponentsAlgorithmsResponse(line 1250) andv2ComponentEncryptionHintsResponse(line 927) still carry the proper "Response message for…" description because the corresponding protoComponentsAlgorithmsResponse/ComponentEncryptionHintsResponsedidn't add adescription:override. The result is inconsistent API documentation — two of the four comparable single-component responses lose their primary description.Suggested fix in the proto (apply symmetrically at proto line 447 and line 927): concatenate the original description with the error-case note, or move the error-case example into the
examplefield's body rather than overridingdescription. For example:- description: "Success example. For error cases, the component block reports the processing status via info_message and info_code. Example: {\"component\":{\"purl\":\"pkg:github/unknown/component\",\"requirement\":\">=1.0.0\",\"version\":\"\",\"algorithms\":[],\"info_message\":\"Component not found in database\",\"info_code\":\"COMPONENT_NOT_FOUND\"},\"status\":{\"status\":\"SUCCESS\",\"message\":\"Request processed\"}}"; + description: "Response message for GetComponentAlgorithms method.\n\nContains cryptographic algorithm information for a single software component including algorithm names, strength classifications, and analysis metadata.\n\nFor error cases, the component block reports the processing status via info_message and info_code. Example: {\"component\":{\"purl\":\"pkg:github/unknown/component\",\"requirement\":\">=1.0.0\",\"version\":\"\",\"algorithms\":[],\"info_message\":\"Component not found in database\",\"info_code\":\"COMPONENT_NOT_FOUND\"},\"status\":{\"status\":\"SUCCESS\",\"message\":\"Request processed\"}}";After regenerating the swagger,
v2ComponentAlgorithmsResponse.descriptionandv2ComponentHintsInRangeResponse.descriptionshould carry both the method description and the error-example text.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json` around lines 882 - 892, The openapiv2_schema.json_schema.description overrides for the proto messages ComponentAlgorithmsResponse and ComponentHintsInRangeResponse are replacing the autogenerated class-level descriptions; update the proto entries (openapiv2_schema.json_schema) for ComponentAlgorithmsResponse and ComponentHintsInRangeResponse to either concatenate the original method description with the error-case note (preserve the existing "Response message for..." text plus the "Success example..." sentence) or remove the description override and place the error-case example into the json_schema.example field instead, then regenerate the swagger so v2ComponentAlgorithmsResponse.description and v2ComponentHintsInRangeResponse.description include the original description plus the example text.
🧹 Nitpick comments (3)
protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json (1)
693-700: Swagger descriptions leak internal proto field numbers — not useful (and potentially confusing) to API consumers.The generated descriptions for
info_message/info_coderead, e.g.,"…Replaces the removed 'error_message' field (position 4)."(lines 695, 699, 746, 750, 780, 784, 850, 854, 954, 958, 1026, 1030, 1110, 1114, 1194, 1198, 1382, 1386, 1486, 1490, 1585, 1589, 1633, 1637, 1730, 1734). Theposition Nphrasing is meaningful for proto schema maintainers reading the.protocomments next to thereservedlines, but for OpenAPI consumers it references a field that (a) no longer exists in the API and (b) has no observable meaning in JSON. This is also inconsistent with how the prior 0.40.0/0.39.0/0.38.0/0.37.0 migrations documented theirinfo_*fields (they did not leak proto numbering into swagger per the CHANGELOG pattern).Suggestion: in the proto, drop the
Replaces the removed \error_message` field (position N).sentence from the field-level comment (keep it only in thereservedblock above where it's wire-format context). Thereserved` comment already provides the migration story for proto readers; the user-facing doc should just describe the current field.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json` around lines 693 - 700, The Swagger descriptions leak proto field numbers by including sentences like "Replaces the removed `error_message` field (position N)" in the field-level comments for info_message and info_code; edit the corresponding .proto field comments (for the info_message and info_code fields across the cryptography messages) to remove the "Replaces the removed ... (position N)" sentence so the OpenAPI generator emits only user-facing descriptions, leaving any migration notes about reserved field positions in the reserved block comments instead.CHANGELOG.md (1)
10-15: Minor wording inconsistency with prior migration entries.Prior
info_message/info_codemigration entries (0.40.0 line 18, 0.39.0 line 24, 0.37.0 line 36) use the phrase "as the definitive fields for reporting the outcome of processing each component". The new 0.41.0Addedline (12) omits that phrasing. Non-blocking, but worth aligning for a consistent changelog voice:-- Added `info_message` and `info_code` fields to all Cryptography response messages (`AlgorithmResponse.Purls`, `ComponentAlgorithms`, ...`ComponentHintsInRangeResponse.Component`) +- Added `info_message` and `info_code` fields to all Cryptography response messages (`AlgorithmResponse.Purls`, `ComponentAlgorithms`, ...`ComponentHintsInRangeResponse.Component`) as the definitive fields for reporting the outcome of processing each componentAlso: the
Addedbullet listsinfo_message/info_codeas new fields, but0.25.0(line 113) had already introducederror_message/error_codefor the same messages. From a user's perspective this release is a rename, not a pure Add+Remove. Consider adding a one-line### Changedhint ("Renamederror_message/error_code→info_message/info_codeacross all cryptography response messages") for readers who skim the changelog — same pattern as 0.36.0 line 42.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@CHANGELOG.md` around lines 10 - 15, Update the 0.41.0 changelog entry to match prior migration wording and clarify this is a rename: in the "Added" bullet for 0.41.0 include the phrase "as the definitive fields for reporting the outcome of processing each component" after mentioning info_message and info_code, and add a new "### Changed" line stating "Renamed error_message/error_code → info_message/info_code across all Cryptography response messages" to indicate this is a rename rather than a pure add/remove; reference the fields info_message, info_code, error_message, error_code and the 0.41.0 version entry when applying the edits.protobuf/scanoss/api/cryptography/v2/README.md (1)
83-100: "Info Codes" section — consider calling out thatINVALID_SEMVER/VERSION_NOT_FOUNDonly apply to version-pinned responses.The table lists all five codes as though any endpoint can return any of them, but the proto
Possible valuescomments explicitly excludeINVALID_SEMVERandVERSION_NOT_FOUNDfrom the range-based responses (AlgorithmsInRangeResponse.Purl,ComponentsAlgorithmsInRangeResponse.Component,ComponentAlgorithmsInRangeResponse.Component,VersionsInRangeResponse.Purl,ComponentsVersionsInRangeResponse.Component,ComponentVersionsInRangeResponse.Component,HintsInRangeResponse.Purl,ComponentsHintsInRangeResponse.Component,ComponentHintsInRangeResponse.Component). A short note under the table would help clients write correct error-handling:| `VERSION_NOT_FOUND` | The specific component version could not be found. | +**Note:** `INVALID_SEMVER` and `VERSION_NOT_FOUND` only apply to endpoints that resolve a specific component version (e.g. `GetComponentAlgorithms`, `GetComponentEncryptionHints`). Range-based endpoints (`*InRange`) will not emit these codes. + **Note**: When a component-level processing issue occurs, the overall response status remains `SUCCESS` since the request was processed. The per-component outcome is carried inside the component block via `info_code`/`info_message`.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@protobuf/scanoss/api/cryptography/v2/README.md` around lines 83 - 100, Update the "Info Codes" section to clarify that the `INVALID_SEMVER` and `VERSION_NOT_FOUND` codes only apply to version-pinned responses by adding a short note under the table stating that these two codes are not returned by range-based responses; reference the specific proto response types listed (AlgorithmsInRangeResponse.Purl, ComponentsAlgorithmsInRangeResponse.Component, ComponentAlgorithmsInRangeResponse.Component, VersionsInRangeResponse.Purl, ComponentsVersionsInRangeResponse.Component, ComponentVersionsInRangeResponse.Component, HintsInRangeResponse.Purl, ComponentsHintsInRangeResponse.Component, ComponentHintsInRangeResponse.Component) so readers know which endpoints exclude those codes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.proto`:
- Around line 351-357: The PR title contains typos: change "witn" to "with" and
"info_massage" to "info_message" so the squash-merge commit subject reads
correctly (e.g., "chore(cryptography): SP-4324 replace error_message/error_code
with info_message/info_code"); verify the enums/fields referenced in the diff
("error_message", "error_code", "info_message", "info_code") are unchanged in
the proto and only the PR title is updated before merging.
---
Outside diff comments:
In `@protobuf/scanoss/api/cryptography/v2/README.md`:
- Around line 405-421: The JSON example under "Component with No Cryptographic
Hints" contains a stray blank line between the "info_code": "NO_INFO" entry and
the closing brace of the "component" object; remove that empty line so the
component object closes immediately after "info_code" (keep the keys "purl",
"version", "hints", "info_message", "info_code" intact) to make the snippet a
clean, copy-paste-able JSON example consistent with other samples.
In `@protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json`:
- Around line 882-892: The openapiv2_schema.json_schema.description overrides
for the proto messages ComponentAlgorithmsResponse and
ComponentHintsInRangeResponse are replacing the autogenerated class-level
descriptions; update the proto entries (openapiv2_schema.json_schema) for
ComponentAlgorithmsResponse and ComponentHintsInRangeResponse to either
concatenate the original method description with the error-case note (preserve
the existing "Response message for..." text plus the "Success example..."
sentence) or remove the description override and place the error-case example
into the json_schema.example field instead, then regenerate the swagger so
v2ComponentAlgorithmsResponse.description and
v2ComponentHintsInRangeResponse.description include the original description
plus the example text.
---
Nitpick comments:
In `@CHANGELOG.md`:
- Around line 10-15: Update the 0.41.0 changelog entry to match prior migration
wording and clarify this is a rename: in the "Added" bullet for 0.41.0 include
the phrase "as the definitive fields for reporting the outcome of processing
each component" after mentioning info_message and info_code, and add a new "###
Changed" line stating "Renamed error_message/error_code → info_message/info_code
across all Cryptography response messages" to indicate this is a rename rather
than a pure add/remove; reference the fields info_message, info_code,
error_message, error_code and the 0.41.0 version entry when applying the edits.
In `@protobuf/scanoss/api/cryptography/v2/README.md`:
- Around line 83-100: Update the "Info Codes" section to clarify that the
`INVALID_SEMVER` and `VERSION_NOT_FOUND` codes only apply to version-pinned
responses by adding a short note under the table stating that these two codes
are not returned by range-based responses; reference the specific proto response
types listed (AlgorithmsInRangeResponse.Purl,
ComponentsAlgorithmsInRangeResponse.Component,
ComponentAlgorithmsInRangeResponse.Component, VersionsInRangeResponse.Purl,
ComponentsVersionsInRangeResponse.Component,
ComponentVersionsInRangeResponse.Component, HintsInRangeResponse.Purl,
ComponentsHintsInRangeResponse.Component,
ComponentHintsInRangeResponse.Component) so readers know which endpoints exclude
those codes.
In `@protobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json`:
- Around line 693-700: The Swagger descriptions leak proto field numbers by
including sentences like "Replaces the removed `error_message` field (position
N)" in the field-level comments for info_message and info_code; edit the
corresponding .proto field comments (for the info_message and info_code fields
across the cryptography messages) to remove the "Replaces the removed ...
(position N)" sentence so the OpenAPI generator emits only user-facing
descriptions, leaving any migration notes about reserved field positions in the
reserved block comments instead.
🪄 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: 9493f1ab-a5de-4489-90b6-58dc9135d6c5
⛔ Files ignored due to path filters (1)
api/cryptographyv2/scanoss-cryptography.pb.gois excluded by!**/*.pb.go
📒 Files selected for processing (4)
CHANGELOG.mdprotobuf/scanoss/api/cryptography/v2/README.mdprotobuf/scanoss/api/cryptography/v2/scanoss-cryptography.protoprotobuf/scanoss/api/cryptography/v2/scanoss-cryptography.swagger.json
isasmendiagus
left a comment
isasmendiagus
left a comment
There was a problem hiding this comment.
Approved with comments, take a look please
…o_massage/info_code in cryptography responses
agustingroh
force-pushed
the
chore/SP-4324-add-info-message-and-info-code-crypto-responses
branch
from
ab920bb to
83fa264
Compare
agustingroh
deleted the
chore/SP-4324-add-info-message-and-info-code-crypto-responses
branch
2 participants
…o_massage/info_code in cryptography responses
Summary by CodeRabbit
Breaking Changes
Documentation
Release