Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new align-proto skill doc, updates a proto dependency in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 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)
Comment |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![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
🧹 Nitpick comments (3)
packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.ts (1)
110-122: Consider returning a typed response or void for consistency.Returning the raw proto response breaks the pattern established by other methods in this class, which always transform responses through the
TcAbacusGrpcTransformer. Even if the response has no meaningful fields, consider:
- Returning
voidif the response truly carries no data- Creating a minimal transformer that returns an empty typed object
This would make the API surface more consistent and decouple consumers from proto internals.
Option: Return void if response is empty
- async createReferrerCode(address: string, code: string) { + async createReferrerCode(address: string, code: string): Promise<void> { const request = TcAbacusPb.CreateReferrerCodeRequest.create({ code, address, }) - const response = await this.executeGrpcCall< + await this.executeGrpcCall< TcAbacusPb.CreateReferrerCodeRequest, TcAbacusPb.CreateReferrerCodeResponse >(request, this.client.createReferrerCode.bind(this.client)) - - return response // has no response fields }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.ts` around lines 110 - 122, The createReferrerCode method currently returns the raw proto response which breaks the class pattern; change it to return a typed void or use the transformer: update the method signature to async createReferrerCode(address: string, code: string): Promise<void> and remove the returned proto object (just await executeGrpcCall(...) and return nothing), or alternatively call a new transformer method on TcAbacusGrpcTransformer (e.g., TcAbacusGrpcTransformer.transformCreateReferrerCodeResponse(response)) and return that typed result so consumers never see proto internals; ensure the method no longer exposes the raw TcAbacusPb.CreateReferrerCodeResponse..claude/skills/align-proto/SKILL.md (2)
150-156: Add blank lines around fenced code blocks.The bash code blocks need blank lines before and after them per markdown formatting standards.
Proposed fix
1. If `@injectivelabs/exceptions` was modified (new module added), build it first: + ```bash cd packages/exceptions && pnpm build ``` + 2. Run `pnpm type-check` in `packages/sdk-ts`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.claude/skills/align-proto/SKILL.md around lines 150 - 156, The markdown contains fenced bash code blocks without blank lines before/after them; update SKILL.md to add an empty line both immediately before and immediately after each triple-backtick fenced code block (the blocks showing "```bash" and closing "```") so the snippets are properly separated from surrounding text and render correctly in Markdown; ensure all three fenced blocks in the listed steps follow this rule.
119-126: Add language specifier to fenced code block.The code block on line 119 lacks a language identifier, which triggers markdown linting warnings.
Proposed fix
-``` +```text 1. types/*.ts (no dependencies) 2. transformers/*Transformer.ts (depends on types) 3. transformers/*StreamTransformer.ts (depends on main transformer) 4. grpc/*Api.ts (depends on types + transformers) 5. grpc_stream/streamV2/*StreamV2.ts (depends on stream transformer) 6. Update all index.ts exports (depends on all above)</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In @.claude/skills/align-proto/SKILL.md around lines 119 - 126, The fenced code
block containing the list starting with "1. types/*.ts (no dependencies)" in
SKILL.md is missing a language specifier which causes markdown lint warnings;
update that fenced block to include a language tag (e.g., add "text" after the
opening triple backticks) so the block becomestext ..., leaving the
content unchanged.</details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In
@packages/sdk-ts/src/client/indexer/transformers/IndexerTcDerivativesStreamTransformer.ts:
- Around line 14-16: The transformer currently returns undefined for booked
orders (the early return in IndexerTcDerivativesStreamTransformer.ts when
order?.state === OrderState.Booked), which violates the stream callback contract
because IndexerGrpcTcDerivativesStreamV2.ts forwards the transformed value to
consumers via callback(transformed). Remove the early return in the transformer
(keep it producing the normal response object) and instead filter booked orders
in the stream caller: update IndexerGrpcTcDerivativesStreamV2.ts where
callback(transformed) is invoked to skip invoking the user callback when
transformed.order?.state === OrderState.Booked (or when transformed indicates a
booked order), following the established pattern used by
IndexerGrpcDerivativeTransformer.grpcOrderHistoriesToOrderHistories and
IndexerGrpcTcDerivativesTransformer.ordersHistoryResponseToOrdersHistory;
alternatively, if you prefer to keep the transformer filtering, return a
response object with order: undefined rather than returning undefined.
Nitpick comments:
In @.claude/skills/align-proto/SKILL.md:
- Around line 150-156: The markdown contains fenced bash code blocks without
blank lines before/after them; update SKILL.md to add an empty line both
immediately before and immediately after each triple-backtick fenced code block
(the blocks showing "bash" and closing "") so the snippets are properly
separated from surrounding text and render correctly in Markdown; ensure all
three fenced blocks in the listed steps follow this rule.- Around line 119-126: The fenced code block containing the list starting with
"1. types/*.ts (no dependencies)" in SKILL.md is missing a language specifier
which causes markdown lint warnings; update that fenced block to include a
language tag (e.g., add "text" after the opening triple backticks) so the block
becomestext ..., leaving the content unchanged.In
@packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.ts:
- Around line 110-122: The createReferrerCode method currently returns the raw
proto response which breaks the class pattern; change it to return a typed void
or use the transformer: update the method signature to async
createReferrerCode(address: string, code: string): Promise and remove the
returned proto object (just await executeGrpcCall(...) and return nothing), or
alternatively call a new transformer method on TcAbacusGrpcTransformer (e.g.,
TcAbacusGrpcTransformer.transformCreateReferrerCodeResponse(response)) and
return that typed result so consumers never see proto internals; ensure the
method no longer exposes the raw TcAbacusPb.CreateReferrerCodeResponse.</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: defaults **Review profile**: CHILL **Plan**: Pro **Run ID**: `656e36cf-1a37-43e7-a763-606f4aa5bca1` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 26c1e07e5700d642c0080e200ac262287d2fce21 and 293a3188c0d8fc1d6769ff80d523dbbc88c4a2b4. </details> <details> <summary>📒 Files selected for processing (6)</summary> * `.claude/skills/align-proto/SKILL.md` * `packages/sdk-ts/package.json` * `packages/sdk-ts/src/client/indexer/transformers/IndexerTcDerivativesStreamTransformer.ts` * `packages/sdk-ts/src/client/tcAbacus/grpc/TcAbacusGrpcApi.ts` * `packages/sdk-ts/src/client/tcAbacus/grpc/transformers/index.ts` * `packages/sdk-ts/src/client/tcAbacus/types/tcAbacus.ts` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
packages/sdk-ts/src/client/indexer/transformers/IndexerTcDerivativesStreamTransformer.ts
Show resolved
Hide resolved
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.
This pull request introduces several improvements and new features to the
tcAbacusclient insdk-ts, focusing on supporting new gRPC methods for referrer code management and eligibility checks. It also includes a dependency update and a minor bugfix in the indexer stream transformer. The most important changes are grouped below.New gRPC Methods for Referrer Codes and Eligibility:
Added three new API methods to
TcAbacusGrpcApi:fetchReferrerEligibility(address)to check if an address is eligible to be a referrer.createReferrerCode(address, code)to create a new referrer code.fetchReferrerCodes(address, cursor?, limit?)to list referrer codes for an address.Implemented corresponding transformation logic in
TcAbacusGrpcTransformer:grpcReferrerEligibilityToReferrerEligibilitygrpcReferrerCodeToReferrerCodegrpcListReferrerCodesToListReferrerCodesExtended type definitions in
tcAbacus.ts:ReferrerEligibilityResponse,ReferrerCode, andListReferrerCodesResponseinterfaces.Dependency Update:
@injectivelabs/tc-abacus-proto-ts-v2dependency from1.18.2to1.18.3inpackage.jsonto support new proto definitions.Bugfixes and Minor Improvements:
IndexerTcDerivativesStreamTransformerto skip orders withOrderState.Bookedwhen transforming stream responses, improving filtering logic. [1] [2]Documentation:
.claude/skills/align-proto/SKILL.mddescribing the workflow, templates, and best practices for aligning proto packages and implementing new gRPC services in the SDK.Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores