refactor: latest gql schema update to account for removal of spot & perp from feature flags #385
Conversation
WalkthroughThe pull request includes significant changes to the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
README.md (1)
Let me analyze the results and generate a final response.
Documentation needs anchor fixes and CHANGELOG cleanup
The verification revealed two areas that need attention:
All internal links in README.md are potentially broken due to incorrect anchor formatting. The headings exist in the document but the anchor references don't match. This needs to be fixed for:
- Installation
- Usage
- Example sections
- Codebase structure
- Development Quick Start
- License
While perpetual markets references have been removed from the main documentation, there are historical references in CHANGELOG.md, which is expected and should be kept as they document the history of changes.
The documentation is otherwise complete and accurate after the feature removal, with no lingering references to spot pools in any markdown files.
🔗 Analysis chain
Line range hint
1-200: Verify documentation completeness after feature removal.The removal of perpetual markets and spot pools examples aligns with the PR objective. However, let's ensure the documentation remains complete and accurate.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that removed features are consistently removed from all documentation. # Test 1: Check for any remaining references to perpetual markets echo "Checking for remaining PERP references..." rg -i "perp|perpetual" -g "*.md" # Test 2: Check for any remaining references to spot pools echo "Checking for remaining spot pool references..." rg -i "spot.?pool" -g "*.md" # Test 3: Verify if there are any broken internal links echo "Checking for potentially broken internal links..." rg -o "\[.*?\]\(#[^)]+\)" README.md | while read -r link; do anchor=$(echo "$link" | sed -E 's/.*\(#([^)]+)\)/\1/') if ! rg -q "^##+ .*$anchor" README.md; then echo "Potential broken link: $link" fi doneLength of output: 5551
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- README.md (1 hunks)
- src/gql/utils/defaultObjects.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
src/gql/utils/defaultObjects.ts (2)
Line range hint
1-1: Verify complete removal of spot & perp related code.The removal of perpetual and spot trading constants aligns with the PR objective. Let's verify there are no lingering references to the removed constants.
Run these checks:
#!/bin/bash # Description: Verify complete removal of spot & perp related code # Test 1: Search for references to removed constants rg -l "defaultPerpMarket|defaultPerpPosition|defaultPool|defaultSpotPool|defaultSpotPoolSwap|defaultMarkPriceCandles|defaultPerpPositionChanges|defaultPerpLeaderboard" # Test 2: Check for spot/perp feature flags rg -A 5 "GQLFeatureFlags" # Test 3: Search for any remaining perp/spot types ast-grep --pattern 'type $_ = { $$$ (perp|spot) $$$ }'
116-116: Verify the impact of commission type change.The change from
[defaultToken]todefaultTokenlooks correct, but we should verify that all consumers ofGQLDistributionCommissionare updated to handle a single token instead of an array.Let's verify the usage:
✅ Verification successful
Let me generate additional verification scripts to specifically check for the commission property usage:
This will help us verify:
- The exact type definition of GQLDistributionCommission
- All places where the commission property is accessed
- Usage of the defaultDistributionCommission object
Let me do one final verification to check the test cases and type usage:
Change from array to single token is safe to proceed
Based on the verification results:
- The
GQLDistributionCommissiontype is used consistently across the codebase- The test cases in
heart-monitor.test.tsusedefaultDistributionCommissiondirectly without accessing array indices- No array-specific access patterns (
.commission[...]) were found in the codebase- The change aligns with the schema updates for removing spot & perp feature flags
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of GQLDistributionCommission to ensure they handle single token # Test 1: Search for direct type usage rg -l "GQLDistributionCommission" # Test 2: Search for defaultDistributionCommission usage rg -l "defaultDistributionCommission" # Test 3: Search for commission property access rg -A 2 "\.commission\["Length of output: 3776
Script:
#!/bin/bash # Search for GQLDistributionCommission type definition and usage ast-grep --pattern 'type GQLDistributionCommission = { $$$ }' # Search for commission property access patterns rg -A 2 "\.commission" # Search for defaultDistributionCommission usage rg -A 2 "defaultDistributionCommission"Length of output: 905
Script:
#!/bin/bash # Check test implementation to understand how commission is used rg -A 5 "testDistributionCommissions" # Find the GQLDistributionCommission type imports and usages rg -A 2 "GQLDistributionCommission"Length of output: 3726
README.md (1)
51-51: LGTM! Minor formatting improvement.The formatting change to the License section link in the Table of Contents improves consistency.
|
Test failure |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/gql/utils/consts.test.ts (2)
Line range hint
18-21: Consider parameterizing the API endpoint URL.The hardcoded testnet URL
"https://hm-graphql.testnet-1.nibiru.fi/query"should ideally be moved to a configuration file or environment variable for better maintainability and flexibility across different environments.}>( [ communityPoolQueryString({}, true, defaultToken), featureFlagsQueryString(true, defaultFeatureFlags), ], - "https://hm-graphql.testnet-1.nibiru.fi/query" + process.env.GRAPHQL_ENDPOINT || "https://hm-graphql.testnet-1.nibiru.fi/query" )
Based on the gathered information, I can now provide the final response:
The review comment correctly identifies missing error scenario tests
The code analysis reveals that while there is error handling in place (particularly in
cleanResponse), the test coverage inconsts.test.tslacks important error scenarios forqueryBatchHandler:
- The
queryBatchHandlerfunction relies ondoGqlQueryandcleanResponsefor error handling, but the test suite only covers successful responses- The implementation in
cleanResponsehandles:
- JSON parsing errors (with catch block)
- Response structure validation
- Non-OK responses
However, the test file (
consts.test.ts) only tests the happy path forqueryBatchHandlerwithout covering:
- Network failures
- Malformed GraphQL responses
- Partial data responses
- Invalid endpoint URLs
The review comment's concerns are valid and the suggested test scenarios would improve the robustness of the test suite.
🔗 Analysis chain
Line range hint
1-61: Verify test coverage for error scenarios.The test suite focuses on happy path scenarios but lacks coverage for potential error cases:
- API failures
- Malformed responses
- Empty or partial feature flags
Consider adding test cases for:
- Network errors
- Invalid GraphQL responses
- Partial feature flags responses
Let me gather more information about the error handling in the codebase, particularly focusing on the GraphQL utilities and their tests.
I'm looking for:
- How queryBatchHandler handles errors internally
- Existing error handling patterns in GraphQL utilities
- The nature of error tests in heart-monitor.test.ts
- Any defined error types that should be tested
This will help us determine if the review comment's concerns about error handling are valid and what specific test cases are missing.
Let me check the implementation of queryBatchHandler and error handling patterns specifically.
I want to verify:
- How queryBatchHandler actually handles errors
- The error handling implementation in cleanResponse
- Any error-related type definitions
This will help us determine if the review comment's concerns about error testing are valid and what specific test coverage is missing.
Let me try one more search to locate these files as they might be in a different directory structure.
I want to locate the actual implementation files and understand the error handling patterns before making a final determination about the review comment's validity regarding error scenario testing.
Let me check the actual implementation of queryBatchHandler and heart-monitor to understand the complete error handling picture.
This will help us understand:
- The actual implementation of queryBatchHandler and its error handling
- How the heart monitor handles GraphQL operations
- The current test coverage in the file under review
This information will allow us to make a final determination about the validity of the review comment regarding error scenario testing.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for error handling tests in GraphQL utility files rg -l 'describe.*error|test.*error|it.*error' src/gql/Length of output: 100
Script:
#!/bin/bash # Check the implementation of queryBatchHandler for error handling ast-grep --pattern 'export const queryBatchHandler = async' # Look for try-catch blocks in GraphQL utility files rg 'try.*catch' src/gql/ -A 5 # Check the actual test file content for error scenarios cat src/gql/heart-monitor/heart-monitor.test.ts # Look for any error types or interfaces related to GraphQL responses rg 'type.*Error|interface.*Error' src/gql/Length of output: 13286
Script:
#!/bin/bash # Check the implementation of queryBatchHandler in the utils cat src/gql/utils/batch.ts # Look for the implementation of cleanResponse function cat src/gql/utils/clean-response.ts # Check if there are any error-related types in types.ts cat src/gql/utils/types.tsLength of output: 269
Script:
#!/bin/bash # Find the relevant files fd -e ts batch fd -e ts "clean-response" # Search for error handling in all TypeScript files under gql rg "catch|throw|Error" src/gql/ --type ts -B 2 -A 2 # Look for the actual implementation files fd -t f . src/gql/Length of output: 2207
Script:
#!/bin/bash # Check the implementation of queryBatchHandler in consts.ts cat src/gql/utils/consts.ts # Check the heart monitor implementation cat src/gql/heart-monitor/heart-monitor.ts # Look at the actual test file under review cat src/gql/utils/consts.test.tsLength of output: 12348
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- src/gql/heart-monitor/heart-monitor.test.ts (2 hunks)
- src/gql/utils/consts.test.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
src/gql/utils/consts.test.ts (1)
40-40: LGTM! Feature flags validation updated correctly.The removal of "perp" and "spot" from the feature flags validation aligns with the PR objectives and changes in other files.
src/gql/heart-monitor/heart-monitor.test.ts (2)
142-142: LGTM: Feature flags update aligned with requirementsThe removal of 'perp' and 'spot' from the feature flags validation is consistent with the PR objectives.
398-398: LGTM: Consistent feature flags validation in batch handlerThe batch handler test maintains consistency with the updated feature flags structure.
|
* chore: update eth proto to latest * refactor: updating to latest graphql schema since featureflags don't include spot & perp (#385) * fix: test * fix: test --------- Co-authored-by: Calico Nino <[email protected]>
* chore: update eth proto to latest * refactor: updating to latest graphql schema since featureflags don't include spot & perp (#385) * fix: test * fix: test * chore: upgrade graphql (#387) * refactor: adding eth & tokenfactory types (#389) * refactor: adding eth & tokenfactory types * chore: remove test file * fix: typo * refactor: wasmclient doesn't need nibi registry --------- Co-authored-by: Calico Nino <[email protected]>
## [6.0.0](v5.0.3...v6.0.0) (2024-12-02) ### Bug Fixes * test ([9d7ff5c](9d7ff5c)) * test ([e0b49c9](e0b49c9)) ### Miscellaneous Chores * update eth proto to latest ([96014f2](96014f2)) * upgrade graphql ([#387](#387)) ([d707e12](d707e12)) ### Code Refactors * adding eth & tokenfactory types ([#389](#389)) ([8c6c34c](8c6c34c)) * updating to latest graphql schema since featureflags don't include spot & perp ([#385](#385)) ([cbb36d7](cbb36d7)) [skip ci]
|
🎉 This PR is included in version 6.0.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
Documentation
Refactor
commissionproperty in thedefaultDistributionCommissionconstant.Tests
featureFlagsfrom six to four, reflecting a change in validation requirements.