Kleros SDK publication config #1704
Conversation
WalkthroughThe pull request introduces several changes across the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SDK
participant GraphQL
User->>SDK: Initiate Action
SDK->>GraphQL: Fetch Data
GraphQL-->>SDK: Return Data
SDK-->>User: Return Result
Possibly related PRs
Suggested reviewers
Poem
🪧 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 (
|
❌ Deploy Preview for kleros-v2-university failed. Why did it fail? →
|
❌ Deploy Preview for kleros-v2-testnet failed. Why did it fail? →
|
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
|
Code Climate has analyzed commit 4e9ad51 and detected 804 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 23
🧹 Outside diff range and nitpick comments (14)
kleros-sdk/src/dataMappings/utils/disputeRequest.ts (1)
1-8: LGTM! Consider enhancing type safety for IDs and URI.The
DisputeRequesttype is well-structured and uses appropriate types for each property. The use of template literal types forarbitrableandarbitratoris a good practice for ensuring type safety with Ethereum addresses.Consider the following suggestions for further improvements:
- For
arbitrableDisputeID,externalDisputeID, andtemplateId, consider using a more specific type thannumber. For instance, you could use a custom type to ensure these are always positive integers:type PositiveInteger = number & { __brand: 'PositiveInteger' };
- For
templateUri, you might want to use a more specific type or add runtime validation to ensure it's a valid URI. For example:type ValidUri = string & { __brand: 'ValidUri' };These suggestions could further enhance type safety and make the code more robust against potential errors.
kleros-sdk/src/dataMappings/utils/populateTemplate.ts (1)
Line range hint
11-14: Address the disabled validation logic.While the comment provides context about the strict validation, disabling it without a proper fix or alternative can lead to potential issues. Consider the following suggestions:
- Implement a less strict validation as an interim solution.
- If possible, provide a timeline for fixing the strict validation.
- Remove the commented-out code and track this issue in your project management tool.
Would you like assistance in implementing a less strict validation or creating a GitHub issue to track this task?
kleros-sdk/src/dataMappings/utils/dataMappingValidators.ts (6)
11-11: LGTM: Function signature updated correctly.The change to use
DataMapping<SubgraphMapping>is consistent with the new type system. However, consider using type guards to improve type safety:export const validateSubgraphMapping = (mapping: DataMapping<SubgraphMapping>): SubgraphMapping => { if (!('endpoint' in mapping)) { throw new Error("Invalid mapping for graphql action."); } return mapping; };This approach eliminates the need for type assertions and provides better type inference.
18-18: LGTM: Function signature updated correctly.The change to use
DataMapping<AbiEventMapping>is consistent with the new type system. Consider applying a similar type guard approach:export const validateAbiEventMapping = (mapping: DataMapping<AbiEventMapping>): AbiEventMapping => { if (!('abi' in mapping) || !('eventFilter' in mapping)) { throw new Error("Invalid mapping for abi/event action."); } return mapping; };This approach improves type safety and eliminates the need for type assertions.
25-25: LGTM: Function signature updated correctly.The change to use
DataMapping<AbiCallMapping>is consistent with the new type system. Consider applying a similar type guard approach:export const validateAbiCallMapping = (mapping: DataMapping<AbiCallMapping>): AbiCallMapping => { if (!('abi' in mapping) || !('args' in mapping)) { throw new Error("Invalid mapping for abi/call action."); } return mapping; };This approach improves type safety and eliminates the need for type assertions.
32-32: LGTM: Function signatures updated correctly.The changes to use
DataMapping<JsonMapping>andDataMapping<FetchIpfsJsonMapping>are consistent with the new type system. Consider applying a similar type guard approach to both functions:For validateJsonMapping:
export const validateJsonMapping = (mapping: DataMapping<JsonMapping>): JsonMapping => { if (!('value' in mapping)) { throw new Error("Invalid mapping for json action."); } return mapping; };For validateFetchIpfsJsonMapping:
export const validateFetchIpfsJsonMapping = (mapping: DataMapping<FetchIpfsJsonMapping>): FetchIpfsJsonMapping => { if (!('ipfsUri' in mapping)) { throw new Error("Invalid mapping for fetch/ipfs/json action."); } return mapping; };These approaches improve type safety and eliminate the need for type assertions.
Also applies to: 39-39
46-46: LGTM: Function signature updated correctly.The change to use
DataMapping<RealityMapping>is consistent with the new type system. Consider applying a similar type guard approach:export const validateRealityMapping = (mapping: DataMapping<RealityMapping>): RealityMapping => { if (mapping.type !== "reality" || typeof mapping.realityQuestionID !== "string") { throw new Error("Invalid mapping for reality action."); } return mapping; };This approach improves type safety and eliminates the need for type assertions. Note that we can directly access
mapping.typeandmapping.realityQuestionIDwithout type assertion because they are part of theRealityMappinginterface.
Line range hint
1-51: Overall assessment: Good changes, with room for improvementThe updates to function signatures using
DataMapping<T>instead ofActionMappingare consistent and improve type safety. However, to further enhance type safety and reduce the reliance on type assertions, consider implementing a generic type guard function:function isValidMapping<T extends object>(mapping: DataMapping<T>, requiredProps: (keyof T)[]): mapping is T { return requiredProps.every(prop => prop in mapping); } // Example usage: export const validateSubgraphMapping = (mapping: DataMapping<SubgraphMapping>): SubgraphMapping => { if (!isValidMapping(mapping, ['endpoint'])) { throw new Error("Invalid mapping for graphql action."); } return mapping; };This approach can be applied to all validation functions, providing a more consistent and type-safe implementation across the file.
kleros-sdk/src/dataMappings/retrieveRealityData.ts (1)
71-71: LGTM: Improved type safety for answers arrayThe explicit type declaration of
answersasAnswer[]enhances type safety and aligns with the new import of theAnswertype. The removal of the loop setting thelastproperty simplifies the code.Consider adding a comment explaining why the
lastproperty is no longer needed or how it's being handled differently now. This would help future maintainers understand the reasoning behind this change.kleros-sdk/test/dataMappings.test.ts (3)
Line range hint
59-78: Address the TODO comment and restore assertions.The addition of the explicit type
DataMapping<SubgraphMapping>improves type safety and is a good change. However, the commented-out assertions need attention.Please review and update the assertions to match the current expected structure of the result. If the structure is still uncertain, consider adding a more descriptive TODO comment explaining what needs to be fixed and why the assertions were removed.
Example of a more descriptive TODO:
// TODO: Update assertions to match the new structure of escrowsData // The structure may have changed due to [reason]. Once confirmed, // uncomment and adjust the following assertions: // expect(result.escrowsData).to.be.an("array"); // result.escrowsData.forEach((escrow) => { // expect(escrow).to.have.property("id"); // expect(escrow).to.have.property("amount"); // expect(escrow).to.have.property("status"); // });
Line range hint
88-102: Update assertion for tokenBalance type.The addition of the explicit type
DataMapping<AbiCallMapping>improves type safety and is a good change. However, the commented-out assertion fortokenBalanceneeds attention.Please review and update the assertion for
tokenBalance. If the expected type has changed frombigint, update the assertion accordingly. If the type is still uncertain, consider adding a more descriptive TODO comment explaining what needs to be fixed and why the assertion was removed.Example of a more descriptive TODO:
// TODO: Confirm the expected type of tokenBalance // The type may have changed from bigint to [new type] due to [reason]. // Once confirmed, uncomment and adjust the following assertion: // expect(result.tokenBalance).to.be.a("[expected type]");
Line range hint
113-132: Verify transferValue type and update assertion.The addition of the explicit type
DataMapping<AbiEventMapping>improves type safety and is a good change. However, the commented-out assertion fortransferValueneeds attention.Please review and update the assertion for
transferValue. If the expected type has changed frombigint, update the assertion accordingly. If the type is still uncertain, consider adding a more descriptive TODO comment explaining what needs to be fixed and why the assertion was removed.Example of a more descriptive TODO:
// TODO: Confirm the expected type of transferValue // The type may have changed from bigint to [new type] due to [reason]. // Once confirmed, uncomment and adjust the following assertion: // expect(result.transferValue).to.be.a("[expected type]");kleros-sdk/src/graphql/gql.ts (2)
5-14: Clarify performance recommendations in comments.The comments explain the performance disadvantages of the
documentsmap. Consider being more explicit about the recommended tools or methods to mitigate these issues, such as specifying the exact Babel or SWC plugins to use in production.
81-207: Consolidate repetitive JSDoc comments.The JSDoc comments are repeated for each overload of the
graphqlfunction, which is unnecessary and bloats the code. Document thegraphqlfunction once, above the overloads, and omit redundant comments to improve readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (25)
- kleros-sdk/.gitignore (1 hunks)
- kleros-sdk/config/v2-disputetemplate/reality/example3/DisputeTemplate.json.mustache (1 hunks)
- kleros-sdk/package.json (2 hunks)
- kleros-sdk/src/dataMappings/actions/callAction.ts (1 hunks)
- kleros-sdk/src/dataMappings/actions/eventAction.ts (1 hunks)
- kleros-sdk/src/dataMappings/actions/fetchIpfsJsonAction.ts (1 hunks)
- kleros-sdk/src/dataMappings/actions/jsonAction.ts (1 hunks)
- kleros-sdk/src/dataMappings/actions/subgraphAction.ts (1 hunks)
- kleros-sdk/src/dataMappings/decoder.ts (2 hunks)
- kleros-sdk/src/dataMappings/executeActions.ts (2 hunks)
- kleros-sdk/src/dataMappings/retrieveRealityData.ts (3 hunks)
- kleros-sdk/src/dataMappings/utils/actionTypes.ts (0 hunks)
- kleros-sdk/src/dataMappings/utils/dataMappingTypes.ts (1 hunks)
- kleros-sdk/src/dataMappings/utils/dataMappingValidators.ts (1 hunks)
- kleros-sdk/src/dataMappings/utils/disputeRequest.ts (1 hunks)
- kleros-sdk/src/dataMappings/utils/populateTemplate.ts (1 hunks)
- kleros-sdk/src/graphql/fragment-masking.ts (1 hunks)
- kleros-sdk/src/graphql/gql.ts (1 hunks)
- kleros-sdk/src/graphql/index.ts (1 hunks)
- kleros-sdk/src/index.ts (1 hunks)
- kleros-sdk/test/dataMappings.test.ts (10 hunks)
- kleros-sdk/test/disputeDetailsSchema.test.ts (1 hunks)
- kleros-sdk/tsconfig-cjs.json (1 hunks)
- kleros-sdk/tsconfig.json (2 hunks)
- tsconfig/base.json (1 hunks)
💤 Files with no reviewable changes (1)
- kleros-sdk/src/dataMappings/utils/actionTypes.ts
✅ Files skipped from review due to trivial changes (6)
- kleros-sdk/.gitignore
- kleros-sdk/src/dataMappings/actions/callAction.ts
- kleros-sdk/src/graphql/index.ts
- kleros-sdk/src/index.ts
- kleros-sdk/test/disputeDetailsSchema.test.ts
- kleros-sdk/tsconfig-cjs.json
🧰 Additional context used
🪛 Biome
kleros-sdk/src/dataMappings/utils/dataMappingTypes.ts
[error] 28-28: Don't use 'BigInt' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'bigint' instead(lint/complexity/noBannedTypes)
[error] 29-29: Don't use 'BigInt' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'bigint' instead(lint/complexity/noBannedTypes)
🔇 Additional comments (43)
kleros-sdk/src/dataMappings/actions/jsonAction.ts (2)
2-2: Verify consistency of the new import path style.The import path for
createResultObjectnow uses a tilde (~) prefix, which is consistent with the standardization mentioned in the summary. This change likely improves module resolution.Let's verify the consistency of this new import style across the codebase:
#!/bin/bash # Description: Check for consistency in using tilde-prefixed import paths # Test 1: Count tilde-prefixed imports echo "Number of tilde-prefixed imports:" rg --type typescript '^\s*import .* from "~' | wc -l # Test 2: Check for any remaining non-tilde absolute imports echo "Checking for non-tilde absolute imports:" rg --type typescript '^\s*import .* from "src/'
1-1: Verify the impact of the import path change.The import path for
JsonMappinghas been updated, which is consistent with the reorganization mentioned in the summary. This change improves the organization of type definitions.Let's verify that this change has been consistently applied across the codebase:
✅ Verification successful
Import path change verified successfully. No remaining imports of
JsonMappingfromactionTypesfound. The new import path is consistently used across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining imports of JsonMapping from actionTypes # Test 1: Search for old import path echo "Checking for old import paths:" rg --type typescript 'from "\.\./utils/actionTypes"' # Test 2: Verify new import path usage echo "Verifying new import path usage:" rg --type typescript 'from "\.\./utils/dataMappingTypes"'Length of output: 344
Script:
#!/bin/bash # Description: Check for any remaining imports of JsonMapping from actionTypes and verify new import paths. # Test 1: Search for old import path without specifying --type echo "Checking for old import paths:" rg 'from "\.\./utils/actionTypes"' --glob '*.ts' # Test 2: Verify new import path usage without specifying --type echo "Verifying new import path usage:" rg 'from "\.\./utils/dataMappingTypes"' --glob '*.ts'Length of output: 474
tsconfig/base.json (5)
4-9: Summary: Modernization of TypeScript configurationThe changes in this file represent a significant modernization of the TypeScript configuration:
- Updating to ES2022 target allows for newer language features.
- Adding ES2023 library provides access to the latest type definitions.
- Switching to Node16 module system and resolution strategy aligns with modern JavaScript practices.
These changes are positive and should lead to more robust and efficient code. However, they may require some adjustments in the codebase and careful consideration of runtime environments.
To ensure a smooth transition, please:
- Verify that all runtime environments support ES2022 features.
- Check that no ES2023 features are used that aren't available in ES2022 runtime.
- Review and update import/export statements as necessary.
- Confirm that all module imports are correctly resolved with the new configuration.
You can use the verification scripts provided in the previous comments to assist with these checks.
8-8: Approved: Update to Node16 module systemSwitching to the Node16 module system is a positive change that aligns with modern JavaScript practices. It provides better support for ESM while maintaining compatibility with CommonJS.
This change may require updates to import/export statements in your codebase. Please verify that all modules are correctly imported/exported. You can use the following script to check for potential issues:
#!/bin/bash # Check for potential import/export issues echo "Checking for CommonJS require statements:" rg '\brequire\s*\(' --type ts echo "Checking for CommonJS module.exports:" rg '\bmodule\.exports\b' --type ts echo "Checking for ESM import statements:" rg '\bimport\s+.*\bfrom\b' --type ts echo "Checking for ESM export statements:" rg '\bexport\s+' --type ts
9-9: Approved: Update to Node16 module resolutionUpdating the
moduleResolutionto "Node16" is consistent with the change to the module system and ensures that TypeScript uses the appropriate algorithm for resolving imports.This change may affect how certain imports are resolved, particularly for packages that have both CommonJS and ESM versions. Please verify that all imports in your project are correctly resolved. You can use the following script to check for potential resolution issues:
#!/bin/bash # Check for potential module resolution issues echo "Checking for relative imports:" rg "from '\.\./|from '\./|from '\.'" --type ts echo "Checking for package imports:" rg "from '[^\.']" --type ts echo "Compiling project to check for resolution errors:" npx tsc --noEmit
4-4: Approved: Update to ES2022 targetUpdating the TypeScript compilation target to ES2022 allows for the use of newer JavaScript features, which can lead to more expressive and efficient code. This is a positive change.
Please ensure that all environments where this code will run (including Node.js versions and browsers if applicable) support ES2022 features. You can verify Node.js compatibility with:
5-7: Approved: Addition of ES2023 libraryAdding the ES2023 library to the TypeScript configuration provides access to the latest type definitions, which is beneficial for code completion and type checking.
While this change is positive, please be cautious when using ES2023 features, as the runtime target is set to ES2022. Ensure that any ES2023 features used are transpiled correctly or have appropriate polyfills. You can verify the TypeScript version and available lib options with:
✅ Verification successful
Verified: Addition of ES2023 library
The inclusion of the ES2023 library in
tsconfig/base.jsonis compatible with the project's TypeScript version (^5.3.3). This addition provides access to the latest type definitions, enhancing code completion and type checking.Please ensure that any ES2023 features used are compatible with the ES2022 target or are appropriately transpiled to prevent runtime issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check TypeScript version and available lib options npx tsc --version npx tsc --showConfig | grep -A 10 '"lib":'Length of output: 98622
Script:
#!/bin/bash # Check TypeScript version directly from node_modules ./node_modules/.bin/tsc --version # Display the "lib" configuration from tsconfig/base.json cat tsconfig/base.json | grep -A 5 '"lib":'Length of output: 295
Script:
#!/bin/bash # Check for TypeScript dependency and its version in package.json grep '"typescript":' package.jsonLength of output: 35
Script:
#!/bin/bash # Check if TypeScript is installed globally tsc --version # Search for TypeScript in package-lock.json and yarn.lock rg '"typescript":' package-lock.json yarn.lock # Search for TypeScript references across the entire codebase rg 'typescript'Length of output: 8894
kleros-sdk/src/dataMappings/utils/populateTemplate.ts (1)
3-3: LGTM! Verify consistency across the project.The change from
./DisputeDetailsValidatorto./disputeDetailsValidatoraligns with the common practice of using lowercase for file names in JavaScript/TypeScript projects.To ensure consistency, please run the following script to check for any remaining uppercase module names:
If the script returns any results, consider renaming those modules to lowercase as well for consistency.
kleros-sdk/src/dataMappings/actions/subgraphAction.ts (3)
2-2: LGTM! Verify import consistency across the codebase.The change in the import path for
SubgraphMappinglooks good. It suggests an improvement in code organization by moving the type to a more specific file.Let's verify that this change is consistent across the codebase:
#!/bin/bash # Description: Check for any remaining imports of SubgraphMapping from actionTypes # Test: Search for imports of SubgraphMapping from actionTypes # Expect: No results, as all should now import from dataMappingTypes rg --type typescript 'import.*SubgraphMapping.*from.*actionTypes' # Test: Confirm the new import is used consistently # Expect: Multiple results showing the new import path rg --type typescript 'import.*SubgraphMapping.*from.*dataMappingTypes'
3-3: LGTM! Verify build configuration and import consistency.The change in the import path for
createResultObjectlooks good. The addition of the tilde (~) suggests a change in the module resolution strategy, likely using a path alias.Let's verify the build configuration and import consistency:
#!/bin/bash # Description: Check build configuration and import consistency # Test: Check for tsconfig.json or similar configuration files # Expect: At least one configuration file that might contain path mappings fd -e json -e jsonc tsconfig # Test: If tsconfig.json exists, check for path mappings # Expect: A path mapping that corresponds to the new import style fd -e json -e jsonc tsconfig --exec jq '.compilerOptions.paths' # Test: Check for consistency of the new import style across the codebase # Expect: Multiple results showing the new import path style rg --type typescript 'from.*~src/' # Test: Check for any remaining old-style imports # Expect: No results, as all should now use the new import style with ~ rg --type typescript 'from.*"src/' -g '!package.json'Please ensure that the project's build configuration (e.g., tsconfig.json) includes the necessary path mappings to support this new import style.
Line range hint
5-22: LGTM! Consider verifying function implementation.The
subgraphActionfunction remains unchanged, which is good as it suggests the core functionality is stable. However, given the changes in imports, it might be worth quickly verifying that the function implementation doesn't need any updates.Let's verify the function implementation:
kleros-sdk/tsconfig.json (2)
Line range hint
3-23: Verify the removal oftargetandmodulecompiler options.The
targetandmodulecompiler options have been removed from the configuration. These options are crucial for determining the output JavaScript version and module system.Please ensure that:
- The removal of these options is intentional.
- The desired
targetandmodulesettings are inherited from the base configuration (@kleros/kleros-v2-tsconfig/base.json).- The inherited or default values align with the project's requirements and the runtime environment where the code will be executed.
Run the following script to check the effective compiler options:
#!/bin/bash # Description: Check the effective compiler options after extending the base configuration. # Test: Use tsc to show the effective configuration echo "Checking effective compiler options:" npx tsc --showConfig | jq '.compilerOptions | {target, module}' # Test: Check the base configuration file echo "Checking base configuration file:" cat node_modules/@kleros/kleros-v2-tsconfig/base.json | jq '.compilerOptions | {target, module}'
2-2: Verify the impact of changing the base configuration.The base configuration has been changed from a React-specific one to a more generic base. This could potentially affect how TypeScript compiles the project.
Please ensure that:
- This change is intentional and aligns with the project's current requirements.
- All necessary compiler options that were previously inherited from the React configuration are now explicitly set if still needed.
- The project's build process and runtime behavior are not negatively affected by this change.
Run the following script to check for any React-specific TypeScript features that might be affected:
kleros-sdk/config/v2-disputetemplate/reality/example3/DisputeTemplate.json.mustache (2)
21-21: Review implications ofarbitratorChainIDchangeThe change of
arbitratorChainIDfrom "421613" to "421614" implies a migration from Arbitrum Goerli to Arbitrum Sepolia testnet. This change has several implications:
- The arbitrator contract is now expected to be on Arbitrum Sepolia instead of Arbitrum Goerli.
- There's a mismatch between
arbitrableChainID(100, which is Gnosis Chain mainnet) andarbitratorChainID(421614, which is Arbitrum Sepolia testnet). This configuration implies cross-chain arbitration, which may or may not be intentional.- The
arbitratorAddressneeds to be verified on the new chain (Arbitrum Sepolia).Please verify the following:
- Confirm that the migration from Arbitrum Goerli to Arbitrum Sepolia is intentional.
- Verify that the cross-chain setup (arbitrable on Gnosis Chain, arbitrator on Arbitrum Sepolia) is correct and supported.
- Check if the
arbitratorAddressis correct on Arbitrum Sepolia. You can use a block explorer like Arbiscan for Sepolia to verify this.#!/bin/bash # Description: Verify the arbitrator address on Arbitrum Sepolia # Test: Check if the arbitrator address exists in other configuration files echo "Checking for occurrences of the arbitrator address across configuration files:" rg '"arbitratorAddress":\s*"0xD08Ab99480d02bf9C092828043f611BcDFEA917b"' config/ -t jsonThis script will help identify if the same arbitrator address is used in other configuration files, which might need similar updates.
21-21: Verify the updatedarbitratorChainIDThe
arbitratorChainIDhas been updated from "421613" to "421614". This change might have implications for the arbitration process.To ensure this change is correct and consistent, please run the following script:
This script will help us verify:
- If there are any remaining instances of the old
arbitratorChainID.- If the new
arbitratorChainIDis used consistently across all dispute templates.- If there are any mismatches between
arbitrableChainIDandarbitratorChainIDthat might need attention.Please review the results and confirm if this change is intentional and consistent across the project.
✅ Verification successful
arbitratorChainID Update Verified
The
arbitratorChainIDhas been successfully updated to "421614" with no remaining instances of the old ID and no mismatches detected.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the arbitratorChainID across all dispute template configurations # Test 1: Check for any remaining instances of the old arbitratorChainID echo "Checking for any remaining instances of arbitratorChainID 421613:" rg '"arbitratorChainID":\s*"421613"' config/v2-disputetemplate -t json # Test 2: Verify the new arbitratorChainID is used consistently echo "Verifying the usage of new arbitratorChainID 421614:" rg '"arbitratorChainID":\s*"421614"' config/v2-disputetemplate -t json # Test 3: Check for any mismatches between arbitrableChainID and arbitratorChainID echo "Checking for mismatches between arbitrableChainID and arbitratorChainID:" rg '"arbitrableChainID":\s*"(\d+)".*"arbitratorChainID":\s*"(\d+)"' config/v2-disputetemplate -t jsonLength of output: 1012
Script:
#!/bin/bash # Description: Locate the DisputeTemplate.json.mustache file and verify arbitratorChainID changes # Step 1: Find the correct path of DisputeTemplate.json.mustache echo "Searching for DisputeTemplate.json.mustache in the repository:" fd "DisputeTemplate\.json\.mustache" . -t f # Step 2: Once the path is found, replace <FOUND_PATH> with the actual path and run the verifications # Example (replace <FOUND_PATH> with the actual path from Step 1): # echo "Checking for any remaining instances of arbitratorChainID 421613 in <FOUND_PATH>:" # rg '"arbitratorChainID":\s*"421613"' <FOUND_PATH> # echo "Verifying the usage of new arbitratorChainID 421614 in <FOUND_PATH>:" # rg '"arbitratorChainID":\s*"421614"' <FOUND_PATH> # echo "Checking for mismatches between arbitrableChainID and arbitratorChainID in <FOUND_PATH>:" # rg '"arbitrableChainID":\s*"(\d+)".*"arbitratorChainID":\s*"(\d+)"' <FOUND_PATH>Length of output: 266
Script:
#!/bin/bash # Description: Verify the arbitratorChainID changes in DisputeTemplate.json.mustache FOUND_PATH="./kleros-sdk/config/v2-disputetemplate/reality/example3/DisputeTemplate.json.mustache" # Test 1: Check for any remaining instances of the old arbitratorChainID echo "Checking for any remaining instances of arbitratorChainID 421613:" rg '"arbitratorChainID":\s*"421613"' "$FOUND_PATH" # Test 2: Verify the new arbitratorChainID is used consistently echo "Verifying the usage of new arbitratorChainID 421614:" rg '"arbitratorChainID":\s*"421614"' "$FOUND_PATH" # Test 3: Check for any mismatches between arbitrableChainID and arbitratorChainID echo "Checking for mismatches between arbitrableChainID and arbitratorChainID:" rg '"arbitrableChainID":\s*"(\d+)".*"arbitratorChainID":\s*"(\d+)"' "$FOUND_PATH"Length of output: 950
kleros-sdk/src/dataMappings/actions/eventAction.ts (2)
Line range hint
6-26: Function logic unchanged and compatible with new imports.The
eventActionfunction's logic remains intact. The changes to the import statements don't appear to affect the function's behavior, as the imported entities are used correctly within the function.
2-4: LGTM! Verify import path resolution.The changes to the import statements look good. The use of the tilde (~) prefix suggests a standardization of import paths, which can improve module resolution and maintainability.
To ensure these changes don't introduce any issues, please verify that the new import paths resolve correctly:
✅ Verification successful
Action Required: Re-verify import paths with corrected ripgrep command.
The previous verification encountered an error with the
rgcommand due to an unrecognized file type. Please re-run the verification using the corrected file type identifier.
Verified: Import paths resolve correctly.
All import statements in
kleros-sdk/src/dataMappings/actions/eventAction.tscorrectly resolve to existing files using the standardized~srcprefix. No issues detected with the import path configurations.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the new import paths resolve correctly # Test: Check if the imported files exist fd -e ts -p "src/dataMappings/utils/dataMappingTypes.ts$" . fd -e ts -p "src/dataMappings/utils/createResultObject.ts$" . fd -e ts -p "src/sdk.ts$" . # Test: Verify no unresolved imports in the file rg --type typescript "import.*from.*~src" kleros-sdk/src/dataMappings/actions/eventAction.tsLength of output: 423
Script:
#!/bin/bash # Description: Verify that the new import paths resolve correctly # Test: Check if the imported files exist fd -e ts -p "src/dataMappings/utils/dataMappingTypes.ts$" . fd -e ts -p "src/dataMappings/utils/createResultObject.ts$" . fd -e ts -p "src/sdk.ts$" . # Test: Verify no unresolved imports in the file rg --type=ts "import.*from.*~src" kleros-sdk/src/dataMappings/actions/eventAction.tsLength of output: 595
kleros-sdk/src/dataMappings/actions/fetchIpfsJsonAction.ts (3)
3-3: LGTM: Import path update is consistent.The import path for
createResultObjecthas been updated to use a tilde prefix, which is consistent with the observed changes in import statements. This modification aligns with the new module resolution strategy.
4-4: LGTM: Import path update is consistent.The import path for
MAX_BYTE_SIZEhas been updated to use a tilde prefix, which is consistent with the observed changes in other import statements. This modification aligns with the new module resolution strategy.
Line range hint
6-38: LGTM: Function implementation is sound and unchanged.The
fetchIpfsJsonActionfunction remains well-implemented:
- It correctly handles different IPFS URI formats.
- It properly validates the response size using the
MAX_BYTE_SIZEconstant.- It checks the content type to ensure JSON data.
- It uses the
createResultObjectfunction to process the fetched data.The function logic is unchanged and continues to work as expected with the updated import paths.
kleros-sdk/package.json (8)
3-3: Verify the significant version bumpThe package version has been updated from 0.1.0 to 2.0.1-3, which is a substantial increase. This typically indicates major changes or a complete overhaul of the package.
Please confirm that this version change aligns with the extent of modifications made to the package and follows your versioning strategy. Also, ensure that all breaking changes are documented in the changelog.
5-8: Improved repository field formatThe repository field has been updated to a more detailed object format, which is a good practice. This change provides better metadata for the package.
11-13: Improved package entry pointsThe updates to the main field and the addition of types and module fields enhance the package's compatibility with different module systems and improve TypeScript support. This is a good improvement.
Please ensure that your build process correctly outputs the files to these locations:
- ./lib/src/index.js
- ./lib/src/index.d.ts
You can verify this by running your build script and checking the output directory structure.
47-47: Appropriate addition of rimrafThe addition of rimraf as a devDependency is consistent with its usage in the new clean script. This is a good practice for ensuring consistent behavior across different environments.
35-36: Improved build and publish scriptsThe updates to the scripts section are beneficial:
- The new clean script ensures a fresh build each time.
- The updated build script incorporates the clean step.
- The new test:run script provides a way to run tests without the UI.
- The publish script simplifies the publishing process.
These changes should improve the development and release workflow.
To ensure these scripts work as intended, you can test them with the following commands:
# Test the build process yarn build # Verify the output in the lib directory ls -R lib # Test the test:run script yarn test:run # Simulate the publish process (this won't actually publish the package) yarn publish --dry-runReview the output of each command to confirm that the scripts are functioning correctly.
Also applies to: 39-40
30-33: Proper publish configurationThe addition of the publishConfig field is appropriate. It ensures that the package will be published as a public package and uses the "latest" tag by default.
To ensure that these settings work as intended, you can use the following command to perform a dry run of the publish process:
npm publish --dry-runThis will simulate the publishing process without actually publishing the package. Review the output to confirm that the access and tag settings are correct.
Line range hint
1-58: Overall package.json improvementsThe changes to the package.json file represent a significant update to the Kleros SDK. Key improvements include:
- Updated version number indicating a major release.
- Enhanced module resolution with main, types, and module fields.
- Better control over published files.
- Improved build and publish scripts.
- Proper publish configuration.
These changes should result in a more maintainable and user-friendly package. However, given the extent of the changes, particularly the version bump, it's crucial to ensure that all modifications are thoroughly tested and documented.
To ensure the package is ready for release:
- Verify that all changes are reflected in the changelog.
- Run a full test suite to ensure no regressions.
- Consider creating a pre-release version for beta testing before final publication.
You can use the following commands to assist with this verification:
# View the changelog (adjust the filename if different) cat CHANGELOG.md # Run all tests yarn test # Create a pre-release version (adjust version as needed) npm version 2.0.1-beta.1 --no-git-tag-version # Perform a dry run of the publish process npm publish --dry-runReview the output of each command to ensure everything is in order before the final release.
14-17: Good practice: Specifying files for publicationThe addition of the files field is a good practice. It explicitly defines which files should be included in the published package, helping to reduce package size and exclude unnecessary files.
To ensure that all necessary files are included and no unintended files are published, you can use the following command to see what files would be included in the published package:
Review the output to confirm that it matches your expectations.
kleros-sdk/src/dataMappings/utils/dataMappingValidators.ts (1)
6-6: LGTM: Import ofDataMappingadded.The addition of
DataMappingto the import statement is consistent with the changes in the function signatures throughout the file.kleros-sdk/src/dataMappings/retrieveRealityData.ts (2)
2-3: LGTM: Enhanced type importsThe addition of
DataMappingandAnswertype imports improves type safety and code clarity. This change aligns well with the subsequent modifications in the file.
9-9: LGTM: Improved type definitions for mappingsThe change from
AbiEventMappingtoDataMapping<AbiEventMapping>for bothquestionMappingandtemplateMappingenhances type safety and flexibility. This modification likely allows for better type inference or additional properties in the mapping structure.To ensure this change doesn't have unintended consequences, please verify its impact on the rest of the codebase:
Also applies to: 47-47
✅ Verification successful
[/run_scripts]
The changes from
AbiEventMappingtoDataMapping<AbiEventMapping>for bothquestionMappingandtemplateMappinghave been successfully verified across the codebase. No issues were found related to these type updates.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other occurrences of AbiEventMapping and DataMapping # Test 1: Search for other occurrences of AbiEventMapping echo "Searching for other occurrences of AbiEventMapping:" rg --type typescript "AbiEventMapping" # Test 2: Search for other occurrences of DataMapping echo "Searching for other occurrences of DataMapping:" rg --type typescript "DataMapping" # Test 3: Check if the DataMapping type is exported correctly echo "Checking if DataMapping type is exported correctly:" rg --type typescript "export.*DataMapping"Length of output: 554
Script:
#!/bin/bash # Description: Re-check for occurrences of AbiEventMapping and DataMapping without specifying file type # Test 1: Search for other occurrences of AbiEventMapping in TypeScript files echo "Searching for other occurrences of AbiEventMapping in .ts and .tsx files:" rg "AbiEventMapping" -g "*.ts" -g "*.tsx" # Test 2: Search for other occurrences of DataMapping in TypeScript files echo "Searching for other occurrences of DataMapping in .ts and .tsx files:" rg "DataMapping" -g "*.ts" -g "*.tsx" # Test 3: Check if the DataMapping type is exported correctly in TypeScript files echo "Checking if DataMapping type is exported correctly in .ts and .tsx files:" rg "export\s+.*DataMapping" -g "*.ts" -g "*.tsx"Length of output: 13588
kleros-sdk/test/dataMappings.test.ts (2)
Line range hint
142-146: LGTM! Type safety improvement.The addition of the explicit type
DataMapping<FetchIpfsJsonMapping>improves type safety and code clarity. This change is beneficial and consistent with the improvements made in other test cases.
207-207: LGTM! Consistent update of arbitratorChainID.The update of the expected
arbitratorChainIDvalue to "421614" in the test result is consistent with the change made in the template string. This ensures that the test correctly validates the updated value.kleros-sdk/src/dataMappings/decoder.ts (2)
3-5: Imports and exports look goodThe addition of
graphqlandDisputeDetailsQueryimports, along with exportingDisputeDetailsQuery, appears correct and consistent with the module structure.
46-46: Good addition of error handling for missingdisputeTemplateAdding a check for the existence of
disputeTemplateenhances robustness by properly handling cases where the template is not found.kleros-sdk/src/dataMappings/utils/dataMappingTypes.ts (2)
1-5: TheJsonMappingtype definition looks good.
42-44: TheRealityMappingtype definition looks good.kleros-sdk/src/graphql/gql.ts (2)
1-3: Approved: Imports are correctly structured.The import statements correctly bring in the necessary modules and types.
212-217: Approved:DocumentTypetype alias is correctly defined.The
DocumentTypetype alias accurately extracts the type from a givenDocumentNode, enhancing type flexibility.kleros-sdk/src/dataMappings/executeActions.ts (4)
14-17: Added necessary type importsThe addition of
DataMappingandDisputeRequestimports ensures proper type definitions are available, enhancing type safety and clarity in the code.
19-19: Updated 'executeAction' function signature for better type safetyThe
executeActionfunction now acceptsDataMapping<any>and an optionalDisputeRequestascontext. This change improves type safety and aligns the function with the updated data mapping types.
35-37: Added validation for 'reality' action type requirementsThe added check ensures that an
arbitrableaddress is present in thecontextwhen executing a 'reality' action. Throwing an error when it's absent prevents runtime errors and ensures the function behaves as expected.
44-45: Updated 'executeActions' function signature and context handlingThe
executeActionsfunction now accepts an optionalDisputeRequestasinitialContext, and initializescontextusingObject.assign({}, initialContext). This change prevents unintended mutations of the originalinitialContextobject and maintains data integrity.
| "esModuleInterop": true, | ||
| "removeComments": true, | ||
| "isolatedModules": true | ||
| "isolatedModules": true, | ||
| "noEmit": false |
There was a problem hiding this comment.
Approve esModuleInterop, but reconsider removing strict checks.
The addition of esModuleInterop is a good change that improves compatibility with CommonJS modules.
However, the removal of strictNullChecks and noUnusedLocals is concerning:
strictNullChecks: Removing this option can lead to null reference errors at runtime.noUnusedLocals: Removing this option can result in code clutter and potential performance issues.
Consider keeping these strict checks enabled to maintain code quality and prevent potential bugs. If they're causing specific issues, it might be better to address those issues directly rather than disabling the checks globally.
If you decide to keep these options disabled, please document the reason for this decision.
| import { FetchIpfsJsonMapping } from "src/dataMappings/utils/actionTypes"; | ||
| import { createResultObject } from "src/dataMappings/utils/createResultObject"; | ||
| import { MAX_BYTE_SIZE } from "src/consts"; | ||
| import { FetchIpfsJsonMapping } from "~src/dataMappings/utils/dataMappingTypes"; |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent usage of src imports without tilde prefix detected.
Several files are still using src/... without the tilde prefix. Please update these import paths to maintain consistency across the codebase:
web/src/hooks/queries/useVotingHistory.tsweb/src/hooks/queries/useUser.tsweb/src/hooks/queries/useTopUsersByCoherenceScore.tsweb/src/pages/Dashboard/index.tsxweb/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsxweb/src/pages/Cases/CasesFetcher.tsxweb/src/utils/uri.tsweb/src/hooks/queries/useJurorStakeDetailsQuery.tsweb/src/hooks/queries/useHomePageQuery.tsweb/src/hooks/queries/useEvidences.tsweb/src/hooks/queries/useDrawQuery.tsweb/src/hooks/queries/useDisputeTemplate.tsweb/src/hooks/queries/useDisputeDetailsQuery.tsweb/src/hooks/queries/useCourtTree.tsweb/src/hooks/queries/useCourtPolicyURI.tsweb/src/hooks/queries/useCourtDetails.tsweb/src/hooks/queries/useClassicAppealQuery.tsweb/src/hooks/queries/useCasesQuery.tsweb/src/hooks/queries/useCounter.tsweb/src/hooks/queries/useAllCasesQuery.tsweb/src/components/Verdict/DisputeTimeline.tsx
🔗 Analysis chain
Verify the consistency of import path changes.
The import path for FetchIpfsJsonMapping has been updated to use a tilde prefix and now points to a different file. This change appears to be part of a larger refactoring effort to standardize import paths and possibly reorganize types.
To ensure consistency across the codebase, please run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of the new import path and check for any remaining old import paths.
# Test 1: Check for the new import path
echo "Occurrences of the new import path:"
rg --type typescript "from ['\"]~src/dataMappings/utils/dataMappingTypes['\"]"
# Test 2: Check for any remaining old import paths
echo "Occurrences of the old import path (if any):"
rg --type typescript "from ['\"]src/dataMappings/utils/actionTypes['\"]"
# Test 3: Check for consistency in using tilde prefix for src imports
echo "Inconsistent src imports without tilde prefix (if any):"
rg --type typescript "from ['\"]src/" | grep -v "from ['\"]~src/"
Length of output: 641
Script:
#!/bin/bash
# Description: Verify the usage of the new import path and check for any remaining old import paths.
# Test 1: Check for the new import path
echo "Occurrences of the new import path:"
rg "from ['\"]~src/dataMappings/utils/dataMappingTypes['\"]" --glob "*.ts" --glob "*.tsx"
# Test 2: Check for any remaining old import paths
echo "Occurrences of the old import path (if any):"
rg "from ['\"]src/dataMappings/utils/actionTypes['\"]" --glob "*.ts" --glob "*.tsx"
# Test 3: Check for consistency in using tilde prefix for src imports
echo "Inconsistent src imports without tilde prefix (if any):"
rg "from ['\"]src/" --glob "*.ts" --glob "*.tsx" | grep -v "from ['\"]~src/"
Length of output: 4386
| @@ -161,7 +174,7 @@ describe("populateTemplate", () => { | |||
| "frontendUrl": "https://example.com", | |||
| "arbitrableChainID": "100", | |||
| "arbitrableAddress": "0x1234567890123456789012345678901234567890", | |||
| "arbitratorChainID": "421613", | |||
| "arbitratorChainID": "421614", | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent arbitratorChainID values detected.
The arbitratorChainID has been updated to "421614" in kleros-sdk/test/dataMappings.test.ts, but the old value "421613" still exists in several other files. This inconsistency may lead to unexpected behavior across different parts of the project.
Files with the old arbitratorChainID:
kleros-sdk/config/v2-disputetemplate/moderate/DisputeDetails.moderate.jsoncontracts/deployments/chiado/ArbitrableExample.jsoncontracts/deployments/chiadoDevnet/ArbitrableExample.json
Please update these files to use the new arbitratorChainID to ensure consistency throughout the project.
🔗 Analysis chain
Verify the updated arbitratorChainID value.
The change of arbitratorChainID from "421613" to "421614" looks intentional. However, it's important to ensure this update is consistent across the project.
To ensure consistency, run the following script:
This script will help identify any inconsistencies in the arbitratorChainID value across the project configuration files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of the new arbitratorChainID value across the project.
# Test: Check for any remaining instances of the old chain ID
rg --type-add 'config:*.{json,js,ts}' --type config '"arbitratorChainID":\s*"421613"' -g '!{node_modules,dist}/**'
# Test: Check for the new chain ID
rg --type-add 'config:*.{json,js,ts}' --type config '"arbitratorChainID":\s*"421614"' -g '!{node_modules,dist}/**'
Length of output: 7183
| export type DataMapping< | ||
| T extends SubgraphMapping | AbiEventMapping | AbiCallMapping | JsonMapping | FetchIpfsJsonMapping | RealityMapping | ||
| > = { | ||
| type: string; | ||
| } & T; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider refining the type property in DataMapping to use literal types.
Defining type as a union of specific string literals can restrict it to known mapping types, enhancing type safety.
Apply this diff to implement the suggestion:
-export type DataMapping<
+export type DataMapping<
T extends SubgraphMapping | AbiEventMapping | AbiCallMapping | JsonMapping | FetchIpfsJsonMapping | RealityMapping
> = {
- type: string;
+ type: 'Subgraph' | 'AbiEvent' | 'AbiCall' | 'Json' | 'FetchIpfsJson' | 'Reality';
} & T;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export type DataMapping< | |
| T extends SubgraphMapping | AbiEventMapping | AbiCallMapping | JsonMapping | FetchIpfsJsonMapping | RealityMapping | |
| > = { | |
| type: string; | |
| } & T; | |
| export type DataMapping< | |
| T extends SubgraphMapping | AbiEventMapping | AbiCallMapping | JsonMapping | FetchIpfsJsonMapping | RealityMapping | |
| > = { | |
| type: 'Subgraph' | 'AbiEvent' | 'AbiCall' | 'Json' | 'FetchIpfsJson' | 'Reality'; | |
| } & T; |
| * The query argument is unknown! | ||
| * Please regenerate the types. |
There was a problem hiding this comment.
Update or remove the placeholder comment about unknown query arguments.
The comment states: "The query argument is unknown! Please regenerate the types." This suggests that the types might be outdated or not properly generated. Ensure the types are up-to-date, and remove or update this comment to reflect the current state.
| export function graphql( | ||
| source: "\n query myCasesPageWhere($id: ID!, $skip: Int, $where: Dispute_filter, $orderDirection: OrderDirection) {\n user(id: $id) {\n disputes(first: 3, skip: $skip, orderBy: lastPeriodChange, orderDirection: $orderDirection, where: $where) {\n ...DisputeDetails\n }\n }\n }\n" | ||
| ): (typeof documents)["\n query myCasesPageWhere($id: ID!, $skip: Int, $where: Dispute_filter, $orderDirection: OrderDirection) {\n user(id: $id) {\n disputes(first: 3, skip: $skip, orderBy: lastPeriodChange, orderDirection: $orderDirection, where: $where) {\n ...DisputeDetails\n }\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query ClassicAppeal($disputeID: ID!, $orderBy: DisputeKitDispute_orderBy, $orderDirection: OrderDirection) {\n dispute(id: $disputeID) {\n period\n court {\n id\n timesPerPeriod\n }\n arbitrated {\n id\n }\n lastPeriodChange\n disputeKitDispute(orderBy: $orderBy, orderDirection: $orderDirection) {\n id\n currentLocalRoundIndex\n localRounds {\n ... on ClassicRound {\n winningChoice\n paidFees\n fundedChoices\n }\n }\n }\n }\n }\n" | ||
| ): (typeof documents)["\n query ClassicAppeal($disputeID: ID!, $orderBy: DisputeKitDispute_orderBy, $orderDirection: OrderDirection) {\n dispute(id: $disputeID) {\n period\n court {\n id\n timesPerPeriod\n }\n arbitrated {\n id\n }\n lastPeriodChange\n disputeKitDispute(orderBy: $orderBy, orderDirection: $orderDirection) {\n id\n currentLocalRoundIndex\n localRounds {\n ... on ClassicRound {\n winningChoice\n paidFees\n fundedChoices\n }\n }\n }\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: '\n query Counter {\n counter(id: "0") {\n id\n cases\n casesRuled\n casesVoting\n casesAppealing\n stakedPNK\n redistributedPNK\n paidETH\n activeJurors\n }\n }\n' | ||
| ): (typeof documents)['\n query Counter {\n counter(id: "0") {\n id\n cases\n casesRuled\n casesVoting\n casesAppealing\n stakedPNK\n redistributedPNK\n paidETH\n activeJurors\n }\n }\n']; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query CourtDetails($id: ID!) {\n court(id: $id) {\n policy\n minStake\n alpha\n numberDisputes\n numberClosedDisputes\n numberAppealingDisputes\n numberStakedJurors\n stake\n paidETH\n paidPNK\n timesPerPeriod\n }\n }\n" | ||
| ): (typeof documents)["\n query CourtDetails($id: ID!) {\n court(id: $id) {\n policy\n minStake\n alpha\n numberDisputes\n numberClosedDisputes\n numberAppealingDisputes\n numberStakedJurors\n stake\n paidETH\n paidPNK\n timesPerPeriod\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query CourtPolicyURI($courtID: ID!) {\n court(id: $courtID) {\n policy\n }\n }\n" | ||
| ): (typeof documents)["\n query CourtPolicyURI($courtID: ID!) {\n court(id: $courtID) {\n policy\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: '\n query CourtTree {\n court(id: "1") {\n name\n id\n children(orderBy: name) {\n name\n id\n children {\n name\n id\n children {\n name\n id\n children {\n name\n id\n children {\n name\n id\n }\n }\n }\n }\n }\n }\n }\n' | ||
| ): (typeof documents)['\n query CourtTree {\n court(id: "1") {\n name\n id\n children(orderBy: name) {\n name\n id\n children {\n name\n id\n children {\n name\n id\n children {\n name\n id\n children {\n name\n id\n }\n }\n }\n }\n }\n }\n }\n']; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query DisputeDetails($disputeID: ID!) {\n dispute(id: $disputeID) {\n court {\n id\n timesPerPeriod\n hiddenVotes\n feeForJuror\n }\n arbitrated {\n id\n }\n period\n ruled\n lastPeriodChange\n currentRuling\n overridden\n tied\n currentRound {\n id\n }\n currentRoundIndex\n }\n }\n" | ||
| ): (typeof documents)["\n query DisputeDetails($disputeID: ID!) {\n dispute(id: $disputeID) {\n court {\n id\n timesPerPeriod\n hiddenVotes\n feeForJuror\n }\n arbitrated {\n id\n }\n period\n ruled\n lastPeriodChange\n currentRuling\n overridden\n tied\n currentRound {\n id\n }\n currentRoundIndex\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query DisputeTemplate($id: ID!) {\n disputeTemplate(id: $id) {\n id\n templateTag\n templateData\n templateDataMappings\n }\n }\n" | ||
| ): (typeof documents)["\n query DisputeTemplate($id: ID!) {\n disputeTemplate(id: $id) {\n id\n templateTag\n templateData\n templateDataMappings\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query Draw($address: String, $disputeID: String, $roundID: String) {\n draws(first: 1000, where: { dispute: $disputeID, juror: $address, round: $roundID }) {\n voteIDNum\n vote {\n ... on ClassicVote {\n commit\n commited\n }\n }\n }\n }\n" | ||
| ): (typeof documents)["\n query Draw($address: String, $disputeID: String, $roundID: String) {\n draws(first: 1000, where: { dispute: $disputeID, juror: $address, round: $roundID }) {\n voteIDNum\n vote {\n ... on ClassicVote {\n commit\n commited\n }\n }\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query Evidences($evidenceGroupID: String) {\n evidences(where: { evidenceGroup: $evidenceGroupID }, orderBy: id, orderDirection: asc) {\n id\n evidence\n sender {\n id\n }\n }\n }\n" | ||
| ): (typeof documents)["\n query Evidences($evidenceGroupID: String) {\n evidences(where: { evidenceGroup: $evidenceGroupID }, orderBy: id, orderDirection: asc) {\n id\n evidence\n sender {\n id\n }\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query HomePage($timeframe: ID) {\n disputes(first: 3) {\n id\n }\n counters(where: { id_gt: $timeframe }) {\n id\n stakedPNK\n paidETH\n redistributedPNK\n activeJurors\n cases\n }\n }\n" | ||
| ): (typeof documents)["\n query HomePage($timeframe: ID) {\n disputes(first: 3) {\n id\n }\n counters(where: { id_gt: $timeframe }) {\n id\n stakedPNK\n paidETH\n redistributedPNK\n activeJurors\n cases\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query JurorStakeDetails($userId: String) {\n jurorTokensPerCourts(where: { juror: $userId }) {\n court {\n id\n name\n }\n staked\n locked\n }\n }\n" | ||
| ): (typeof documents)["\n query JurorStakeDetails($userId: String) {\n jurorTokensPerCourts(where: { juror: $userId }) {\n court {\n id\n name\n }\n staked\n locked\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query TopUsersByCoherenceScore($first: Int!, $orderBy: User_orderBy, $orderDirection: OrderDirection) {\n users(first: $first, orderBy: $orderBy, orderDirection: $orderDirection) {\n id\n coherenceScore\n totalCoherent\n totalResolvedDisputes\n }\n }\n" | ||
| ): (typeof documents)["\n query TopUsersByCoherenceScore($first: Int!, $orderBy: User_orderBy, $orderDirection: OrderDirection) {\n users(first: $first, orderBy: $orderBy, orderDirection: $orderDirection) {\n id\n coherenceScore\n totalCoherent\n totalResolvedDisputes\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n fragment UserDetails on User {\n totalDisputes\n totalResolvedDisputes\n totalAppealingDisputes\n totalCoherent\n coherenceScore\n tokens {\n court {\n id\n name\n }\n }\n shifts {\n pnkAmount\n ethAmount\n }\n }\n" | ||
| ): (typeof documents)["\n fragment UserDetails on User {\n totalDisputes\n totalResolvedDisputes\n totalAppealingDisputes\n totalCoherent\n coherenceScore\n tokens {\n court {\n id\n name\n }\n }\n shifts {\n pnkAmount\n ethAmount\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query User($address: ID!) {\n user(id: $address) {\n disputes(orderBy: lastPeriodChange) {\n id\n }\n ...UserDetails\n }\n }\n" | ||
| ): (typeof documents)["\n query User($address: ID!) {\n user(id: $address) {\n disputes(orderBy: lastPeriodChange) {\n id\n }\n ...UserDetails\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query UserDisputeFilter($address: ID!, $where: Dispute_filter) {\n user(id: $address) {\n disputes(orderBy: lastPeriodChange, where: $where) {\n id\n }\n ...UserDetails\n }\n }\n" | ||
| ): (typeof documents)["\n query UserDisputeFilter($address: ID!, $where: Dispute_filter) {\n user(id: $address) {\n disputes(orderBy: lastPeriodChange, where: $where) {\n id\n }\n ...UserDetails\n }\n }\n"]; | ||
| /** | ||
| * The graphql function is used to parse GraphQL queries into a document that can be used by GraphQL clients. | ||
| */ | ||
| export function graphql( | ||
| source: "\n query VotingHistory($disputeID: ID!) {\n dispute(id: $disputeID) {\n id\n createdAt\n rounds {\n nbVotes\n court {\n id\n name\n }\n timeline\n }\n disputeKitDispute {\n localRounds {\n ... on ClassicRound {\n winningChoice\n totalVoted\n justifications {\n id\n juror {\n id\n }\n choice\n reference\n }\n }\n }\n }\n }\n }\n" | ||
| ): (typeof documents)["\n query VotingHistory($disputeID: ID!) {\n dispute(id: $disputeID) {\n id\n createdAt\n rounds {\n nbVotes\n court {\n id\n name\n }\n timeline\n }\n disputeKitDispute {\n localRounds {\n ... on ClassicRound {\n winningChoice\n totalVoted\n justifications {\n id\n juror {\n id\n }\n choice\n reference\n }\n }\n }\n }\n }\n }\n"]; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reduce duplication in graphql function overloads.
The numerous overloads of the graphql function for each specific query increase code complexity and hinder maintainability. Consider automating the generation of these overloads using code generation tools or refactoring to minimize repetition.
| export function graphql(source: string) { | ||
| return (documents as any)[source] ?? {}; | ||
| } |
There was a problem hiding this comment.
Enhance type safety in the graphql function return value.
Returning an empty object {} when the query is not found may lead to runtime errors if the calling code expects a specific type. Consider throwing an error or returning a clearly defined fallback type to ensure safer usage.
Apply this change to throw an error for unknown queries:
export function graphql(source: string) {
- return (documents as any)[source] ?? {};
+ const document = (documents as any)[source];
+ if (!document) {
+ throw new Error(`GraphQL query "${source}" is not found in documents map.`);
+ }
+ return document;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function graphql(source: string) { | |
| return (documents as any)[source] ?? {}; | |
| } | |
| export function graphql(source: string) { | |
| const document = (documents as any)[source]; | |
| if (!document) { | |
| throw new Error(`GraphQL query "${source}" is not found in documents map.`); | |
| } | |
| return document; | |
| } |
| const documents = { | ||
| "\n query AllCases {\n counter(id: 0) {\n cases\n casesRuled\n }\n }\n": types.AllCasesDocument, | ||
| "\n fragment DisputeDetails on Dispute {\n id\n arbitrated {\n id\n }\n court {\n id\n policy\n feeForJuror\n timesPerPeriod\n }\n period\n lastPeriodChange\n }\n": | ||
| types.DisputeDetailsFragmentDoc, | ||
| "\n query CasesPageWhere($skip: Int, $where: Dispute_filter, $orderDirection: OrderDirection, $first: Int) {\n disputes(first: $first, skip: $skip, orderBy: lastPeriodChange, orderDirection: $orderDirection, where: $where) {\n ...DisputeDetails\n }\n }\n": | ||
| types.CasesPageWhereDocument, | ||
| "\n query CasesPage($skip: Int, $orderDirection: OrderDirection, $first: Int) {\n disputes(first: $first, skip: $skip, orderBy: lastPeriodChange, orderDirection: $orderDirection) {\n ...DisputeDetails\n }\n }\n": | ||
| types.CasesPageDocument, | ||
| "\n query MyCases($id: ID!, $skip: Int, $orderDirection: OrderDirection) {\n user(id: $id) {\n disputes(first: 3, skip: $skip, orderBy: lastPeriodChange, orderDirection: $orderDirection) {\n ...DisputeDetails\n }\n }\n }\n": | ||
| types.MyCasesDocument, | ||
| "\n query myCasesPageWhere($id: ID!, $skip: Int, $where: Dispute_filter, $orderDirection: OrderDirection) {\n user(id: $id) {\n disputes(first: 3, skip: $skip, orderBy: lastPeriodChange, orderDirection: $orderDirection, where: $where) {\n ...DisputeDetails\n }\n }\n }\n": | ||
| types.MyCasesPageWhereDocument, | ||
| "\n query ClassicAppeal($disputeID: ID!, $orderBy: DisputeKitDispute_orderBy, $orderDirection: OrderDirection) {\n dispute(id: $disputeID) {\n period\n court {\n id\n timesPerPeriod\n }\n arbitrated {\n id\n }\n lastPeriodChange\n disputeKitDispute(orderBy: $orderBy, orderDirection: $orderDirection) {\n id\n currentLocalRoundIndex\n localRounds {\n ... on ClassicRound {\n winningChoice\n paidFees\n fundedChoices\n }\n }\n }\n }\n }\n": | ||
| types.ClassicAppealDocument, | ||
| '\n query Counter {\n counter(id: "0") {\n id\n cases\n casesRuled\n casesVoting\n casesAppealing\n stakedPNK\n redistributedPNK\n paidETH\n activeJurors\n }\n }\n': | ||
| types.CounterDocument, | ||
| "\n query CourtDetails($id: ID!) {\n court(id: $id) {\n policy\n minStake\n alpha\n numberDisputes\n numberClosedDisputes\n numberAppealingDisputes\n numberStakedJurors\n stake\n paidETH\n paidPNK\n timesPerPeriod\n }\n }\n": | ||
| types.CourtDetailsDocument, | ||
| "\n query CourtPolicyURI($courtID: ID!) {\n court(id: $courtID) {\n policy\n }\n }\n": | ||
| types.CourtPolicyUriDocument, | ||
| '\n query CourtTree {\n court(id: "1") {\n name\n id\n children(orderBy: name) {\n name\n id\n children {\n name\n id\n children {\n name\n id\n children {\n name\n id\n children {\n name\n id\n }\n }\n }\n }\n }\n }\n }\n': | ||
| types.CourtTreeDocument, | ||
| "\n query DisputeDetails($disputeID: ID!) {\n dispute(id: $disputeID) {\n court {\n id\n timesPerPeriod\n hiddenVotes\n feeForJuror\n }\n arbitrated {\n id\n }\n period\n ruled\n lastPeriodChange\n currentRuling\n overridden\n tied\n currentRound {\n id\n }\n currentRoundIndex\n }\n }\n": | ||
| types.DisputeDetailsDocument, | ||
| "\n query DisputeTemplate($id: ID!) {\n disputeTemplate(id: $id) {\n id\n templateTag\n templateData\n templateDataMappings\n }\n }\n": | ||
| types.DisputeTemplateDocument, | ||
| "\n query Draw($address: String, $disputeID: String, $roundID: String) {\n draws(first: 1000, where: { dispute: $disputeID, juror: $address, round: $roundID }) {\n voteIDNum\n vote {\n ... on ClassicVote {\n commit\n commited\n }\n }\n }\n }\n": | ||
| types.DrawDocument, | ||
| "\n query Evidences($evidenceGroupID: String) {\n evidences(where: { evidenceGroup: $evidenceGroupID }, orderBy: id, orderDirection: asc) {\n id\n evidence\n sender {\n id\n }\n }\n }\n": | ||
| types.EvidencesDocument, | ||
| "\n query HomePage($timeframe: ID) {\n disputes(first: 3) {\n id\n }\n counters(where: { id_gt: $timeframe }) {\n id\n stakedPNK\n paidETH\n redistributedPNK\n activeJurors\n cases\n }\n }\n": | ||
| types.HomePageDocument, | ||
| "\n query JurorStakeDetails($userId: String) {\n jurorTokensPerCourts(where: { juror: $userId }) {\n court {\n id\n name\n }\n staked\n locked\n }\n }\n": | ||
| types.JurorStakeDetailsDocument, | ||
| "\n query TopUsersByCoherenceScore($first: Int!, $orderBy: User_orderBy, $orderDirection: OrderDirection) {\n users(first: $first, orderBy: $orderBy, orderDirection: $orderDirection) {\n id\n coherenceScore\n totalCoherent\n totalResolvedDisputes\n }\n }\n": | ||
| types.TopUsersByCoherenceScoreDocument, | ||
| "\n fragment UserDetails on User {\n totalDisputes\n totalResolvedDisputes\n totalAppealingDisputes\n totalCoherent\n coherenceScore\n tokens {\n court {\n id\n name\n }\n }\n shifts {\n pnkAmount\n ethAmount\n }\n }\n": | ||
| types.UserDetailsFragmentDoc, | ||
| "\n query User($address: ID!) {\n user(id: $address) {\n disputes(orderBy: lastPeriodChange) {\n id\n }\n ...UserDetails\n }\n }\n": | ||
| types.UserDocument, | ||
| "\n query UserDisputeFilter($address: ID!, $where: Dispute_filter) {\n user(id: $address) {\n disputes(orderBy: lastPeriodChange, where: $where) {\n id\n }\n ...UserDetails\n }\n }\n": | ||
| types.UserDisputeFilterDocument, | ||
| "\n query VotingHistory($disputeID: ID!) {\n dispute(id: $disputeID) {\n id\n createdAt\n rounds {\n nbVotes\n court {\n id\n name\n }\n timeline\n }\n disputeKitDispute {\n localRounds {\n ... on ClassicRound {\n winningChoice\n totalVoted\n justifications {\n id\n juror {\n id\n }\n choice\n reference\n }\n }\n }\n }\n }\n }\n": | ||
| types.VotingHistoryDocument, | ||
| }; |
There was a problem hiding this comment.
Replace full query strings with identifiers as keys in the documents map.
Using entire GraphQL query strings as keys can lead to maintenance issues. Any formatting changes or minor edits to the queries will break the key mapping. Consider using unique identifiers or hashing the query strings to improve robustness and maintainability.
Apply this refactor to use identifiers as keys:
-const documents = {
- "\n query AllCases {\n counter(id: 0) {\n cases\n casesRuled\n }\n }\n": types.AllCasesDocument,
+const documents = {
+ "AllCases": types.AllCasesDocument,
...
};Update the graphql function to retrieve documents by these identifiers:
-export function graphql(source: string) {
- return (documents as any)[source] ?? {};
+export function graphql(key: string) {
+ return documents[key] ?? {};
}Committable suggestion was skipped due to low confidence.
|
Too old, superseded by #1703 |
PR-Codex overview
This PR primarily focuses on restructuring and enhancing the
kleros-sdkproject by updating type definitions, improving type safety, and modifying the GraphQL integration. It also updates configuration files and modifies various action imports for better clarity and organization.Detailed summary
DisputeRequesttype indisputeRequest.ts.dataMappingTypes.tsto enhance mapping types.~for better clarity.executeActionto utilizeDataMappingandDisputeRequest.graphqlintegration ingraphql/index.ts.arbitratorChainIDinDisputeTemplate.json.mustache.tsconfigfiles for better module resolution.package.jsonto2.0.1-3.Summary by CodeRabbit
New Features
arbitratorChainIDin dispute templates to improve compatibility.Bug Fixes
Documentation
Chores