feat(kleros-sdk): extra-evidences-schema #2210
Conversation
❌ Deploy Preview for kleros-v2-neo failed. Why did it fail? →
|
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
❌ Deploy Preview for kleros-v2-testnet-devtools failed. Why did it fail? →
|
WalkthroughAdds TxHash and Evidence zod schemas to the SDK, extends DisputeDetailsSchema with Changes
Sequence Diagram(s)sequenceDiagram
participant Browser as Browser (Evidence page)
participant Hook as usePopulatedDisputeData
participant SDK as kleros-sdk (disputeDetailsSchema)
participant UI as EvidenceCard
Browser->>Hook: request populated dispute data (id, arbitrable?)
Hook->>SDK: fetch & validate dispute data (includes extraEvidences)
SDK-->>Hook: validated dispute data (extraEvidences, tx hash checks)
Hook-->>Browser: return populated dispute data
Browser->>UI: render EvidenceCard for each evidence in extraEvidences
UI-->>Browser: conditional display (sender/index/tx link/timestamp)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learnings📚 Learning: 2024-10-14T15:31:27.821ZApplied to files:
🧬 Code graph analysis (1)web/src/components/EvidenceCard.tsx (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
🔇 Additional comments (2)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
kleros-sdk/test/disputeDetailsSchema.test.ts (2)
67-80: Test suite implementation looks good.The test logic correctly validates both success and failure cases with appropriate error message checking.
Minor naming observation: The test suite is named "txHashSchema" (camelCase) while the imported schema is "TxHashSchema" (PascalCase). Consider using "TxHashSchema" for consistency, though this is a minor style point.
Optional: Align test suite naming with import
- describe("txHashSchema", () => { + describe("TxHashSchema", () => {
131-133: Consider adding tests for the extended DisputeDetailsSchema.With the addition of the
extraEvidencesfield toDisputeDetailsSchema, this would be a good opportunity to implement tests for the full schema including the new evidence validation.Would you like me to help generate test cases for the
disputeDetailsSchemaincluding validation of the newextraEvidencesfield with various evidence objects?kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts (1)
26-28: Transaction hash validation is correct.The schema properly validates Ethereum transaction hashes (32 bytes = 64 hex characters + "0x" prefix = 66 total length).
The implementation reuses
isHexIdand adds a length check, which is functionally correct and maintains consistency with existing patterns in the codebase.Optional: Performance micro-optimization
If you prefer a single-pass validation, you could create a dedicated regex:
export const isTxHash = (str: string): boolean => /^0x[a-fA-F0-9]{64}$/.test(str); export const TxHashSchema = z.string().refine(isTxHash, { message: "Provided transaction hash is invalid.", });However, the current approach is perfectly acceptable and maintains consistency with the existing
isHexIdutility.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.tskleros-sdk/test/disputeDetailsSchema.test.ts
🧰 Additional context used
🧬 Code graph analysis (1)
kleros-sdk/test/disputeDetailsSchema.test.ts (1)
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts (1)
TxHashSchema(26-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Analyze (javascript)
- GitHub Check: hardhat-tests
🔇 Additional comments (4)
kleros-sdk/test/disputeDetailsSchema.test.ts (2)
6-6: LGTM!The import is correctly added and matches the export from the implementation file.
33-48: Comprehensive test data!The test data effectively covers the validation requirements with proper edge cases including length variations, prefix issues, and invalid hex characters.
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts (2)
60-70: Well-structured evidence schema following ERC-1497.The implementation correctly follows the ERC-1497 Evidence Standard for core fields while adding appropriate court UI-specific extensions. The use of existing schemas (
TxHashSchema,ethAddressOrEnsNameSchema) for validation ensures consistency across the codebase.
91-91: Good use of default value for array field.The
extraEvidencesfield is properly defined as an array ofEvidenceSchemawith a sensible default of an empty array. This defensive approach prevents undefined/null issues in consuming code.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (2)
98-98: Consider handling error and loading states.While the hook internally handles undefined parameters safely (as per learnings), consider destructuring and handling
errorandisLoadingstates fromusePopulatedDisputeDatato provide better user feedback when fetching arbitrable evidences fails or is in progress.🔎 Suggested enhancement
- const { data: disputeData } = usePopulatedDisputeData(id, arbitrable); + const { data: disputeData, error, isLoading } = usePopulatedDisputeData(id, arbitrable);Then add appropriate UI feedback based on these states.
134-140: Use unique identifier for key instead of array index.Using
indexas the key can cause issues if the array order changes. If the evidence items have a unique identifier (e.g.,id,transactionHash, or a combination), use that instead.🔎 Suggested improvement
- {arbitrableEvidences.map(({ name, description, fileURI, sender, timestamp, transactionHash }, index) => ( + {arbitrableEvidences.map(({ name, description, fileURI, sender, timestamp, transactionHash }) => ( <EvidenceCard - key={index} + key={transactionHash || `${sender}-${timestamp}`} evidence="" {...{ sender, timestamp, transactionHash, name, description, fileURI }} />Adjust the key based on which fields are guaranteed to be unique and present.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
web/src/components/EvidenceCard.tsxweb/src/pages/Cases/CaseDetails/Evidence/index.tsx
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2024-10-14T15:31:27.821Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/pages/Cases/CaseDetails/Evidence/index.tsx:79-79
Timestamp: 2024-10-14T15:31:27.821Z
Learning: In the `EvidenceSearch` component, an undefined `evidenceGroup` prop is expected and handled within the function.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsxweb/src/components/EvidenceCard.tsx
📚 Learning: 2024-10-14T13:58:25.708Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-05-09T13:39:15.086Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx:64-0
Timestamp: 2025-05-09T13:39:15.086Z
Learning: In PersonFields.tsx, the useEffect hook for address validation intentionally uses an empty dependency array to run only once on component mount. This is specifically designed for the dispute duplication flow when aliasesArray is already populated with addresses that need initial validation.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-05-15T06:50:40.859Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-09-09T13:33:46.896Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 2117
File: web/src/components/DisputeFeatures/Features/GatedErc1155.tsx:51-66
Timestamp: 2025-09-09T13:33:46.896Z
Learning: The `setDisputeData` function in `NewDisputeContext` at `web/src/context/NewDisputeContext.tsx` has signature `(disputeData: IDisputeData) => void` and only accepts direct state values, not functional updates like standard React state setters. It cannot be used with the pattern `setDisputeData((prev) => ...)`.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-09T10:22:41.474Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90
Timestamp: 2024-10-09T10:22:41.474Z
Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-09T10:23:39.325Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:98-103
Timestamp: 2024-10-09T10:23:39.325Z
Learning: In `SelectArbitrable.tsx` of the web-devtools project (React/TypeScript), direct DOM manipulation using `child.click()` is acceptable when considered reasonably safe.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
🧬 Code graph analysis (2)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (3)
web/src/hooks/useSpamEvidence.ts (1)
useSpamEvidence(33-43)web/src/hooks/queries/usePopulatedDisputeData.ts (1)
usePopulatedDisputeData(30-84)web/src/components/Divider.tsx (1)
Divider(3-10)
web/src/components/EvidenceCard.tsx (2)
web/src/utils/index.ts (1)
isUndefined(5-6)web/src/utils/date.ts (1)
formatDate(23-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Analyze (javascript)
- GitHub Check: hardhat-tests
🔇 Additional comments (6)
web/src/components/EvidenceCard.tsx (3)
188-192: LGTM! Interface changes support arbitrable evidences.Making
sender,index, andtransactionHashoptional appropriately supports the new arbitrable evidence flow where these fields may not be available.
215-215: LGTM! Rendering guards appropriately handle optional fields.The conditional rendering logic correctly uses
isUndefinedchecks to prevent displaying incomplete UI elements when optional props are missing.Also applies to: 230-239
207-209: EnsuregetTxnExplorerLinkreturnsundefinedfor empty hash input to properly guard against invalid URLs.The function returns
${getChain(DEFAULT_CHAIN)?.blockExplorers?.default.url}/tx/${hash}, which always produces a truthy string—even whenhashis empty. This means the guard at line 235 (isUndefined(transactionExplorerLink)) won't prevent rendering an incomplete URL likehttps://arbitrum.io/tx/. Either modifygetTxnExplorerLinkto returnundefinedfor empty strings, or add an explicit check before calling it.web/src/pages/Cases/CaseDetails/Evidence/index.tsx (3)
88-91: LGTM! Interface appropriately extends Evidence component.The
IEvidenceinterface with an optionalarbitrableaddress prop correctly enables the component to display arbitrable-provided evidences while maintaining backward compatibility.
119-119: LGTM! Safe extraction of arbitrable evidences.The optional chaining appropriately handles cases where
disputeDatais undefined orextraEvidencesis not present.
134-134: Field alignment is correct. All destructured fields (name,description,fileURI,sender,timestamp,transactionHash) are defined in EvidenceSchema and match their expected types. The optionalfileTypeExtensionfield is not used in this component, which is intentional.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @web/src/pages/Cases/CaseDetails/Evidence/index.tsx:
- Around line 183-194: The rendering condition currently checks
data?.evidences.length !== 0 but maps over evidences?.realEvidences, so if all
items are spam the container and heading render with no cards; change the
condition to guard on evidences?.realEvidences (e.g.,
evidences?.realEvidences?.length > 0) before rendering EvidenceCardContainer and
the map, ensuring the UI only shows the heading/container when there are actual
non‑spam evidences to display.
🧹 Nitpick comments (4)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (4)
188-188: Consider using unique evidence IDs as keys instead of timestamp.Lines 188 and 213 use
timestampas the key prop. While timestamps are typically unique, using a dedicated unique identifier (likeevidence.idif available from the graph query) would be more robust and prevent potential collisions if multiple evidences share the same timestamp.#!/bin/bash # Check if evidence objects have an id field available echo "=== Checking evidence structure from useEvidences query ===" rg -n "useEvidences" --type ts -A 20 | head -50Also applies to: 213-213
163-165: Consider extracting tooltip text to a constant.The tooltip message on line 163 is quite long. Consider extracting it to a constant at the top of the file for better maintainability and potential reuse.
♻️ Suggested refactor
At the top of the file, after imports:
const JUSTIFICATIONS_TOOLTIP = "Justifications are submitted by one party before the case is created and explain their initial position or reasons for initiating the dispute.";Then use it:
- <WithHelpTooltip tooltipMsg="Justifications are submitted by one party before the case is created and explain their initial position or reasons for initiating the dispute."> + <WithHelpTooltip tooltipMsg={JUSTIFICATIONS_TOOLTIP}>
167-173: Consider using a stable key instead of array index.Line 169 uses
key={index}which uses the array position. This can cause issues if the evidence list order changes or items are modified. The code below (line 188) demonstrates a better pattern by usingkey={timestamp}for real evidences. Consider adopting the same approach for arbitrable evidences.Additionally, line 170 passes
evidence=""toEvidenceCard. While this works because the component renders thedescriptionfield when present (and arbitrable evidences always have a name and description), consider passing theevidencefield from the data if available, or usingundefinedinstead of an empty string to make the intent clearer.
121-128: Consider standardizing howidis passed to the hooks for code clarity.All three hooks safely handle undefined parameters through
isEnabledguards. However, the call sites are inconsistent:usePopulatedDisputeDatareceives bareid, whileuseSpamEvidenceanduseEvidencesreceiveid!. Since bothusePopulatedDisputeDataanduseEvidencesaccept optional parameters and handle undefined gracefully, consider passingidwithout the non-null assertion to all three hooks for consistency. Alternatively, ifidis guaranteed to exist in this route context, ensure all type signatures reflect that.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
🧰 Additional context used
🧠 Learnings (9)
📚 Learning: 2024-10-14T15:31:27.821Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/pages/Cases/CaseDetails/Evidence/index.tsx:79-79
Timestamp: 2024-10-14T15:31:27.821Z
Learning: In the `EvidenceSearch` component, an undefined `evidenceGroup` prop is expected and handled within the function.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-14T13:58:25.708Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-05-09T13:39:15.086Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx:64-0
Timestamp: 2025-05-09T13:39:15.086Z
Learning: In PersonFields.tsx, the useEffect hook for address validation intentionally uses an empty dependency array to run only once on component mount. This is specifically designed for the dispute duplication flow when aliasesArray is already populated with addresses that need initial validation.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-05-15T06:50:40.859Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-09-09T13:33:46.896Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 2117
File: web/src/components/DisputeFeatures/Features/GatedErc1155.tsx:51-66
Timestamp: 2025-09-09T13:33:46.896Z
Learning: The `setDisputeData` function in `NewDisputeContext` at `web/src/context/NewDisputeContext.tsx` has signature `(disputeData: IDisputeData) => void` and only accepts direct state values, not functional updates like standard React state setters. It cannot be used with the pattern `setDisputeData((prev) => ...)`.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-09T10:22:41.474Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90
Timestamp: 2024-10-09T10:22:41.474Z
Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-09T10:23:39.325Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:98-103
Timestamp: 2024-10-09T10:23:39.325Z
Learning: In `SelectArbitrable.tsx` of the web-devtools project (React/TypeScript), direct DOM manipulation using `child.click()` is acceptable when considered reasonably safe.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-09-12T14:45:48.019Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1687
File: web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx:67-84
Timestamp: 2024-09-12T14:45:48.019Z
Learning: In the `submitEvidence` function of the SubmitEvidenceModal component, the try-catch block is specifically designed to handle errors from the `constructEvidence` function, which may throw an Error if file upload fails. This ensures that the file is successfully uploaded before proceeding with evidence submission and transaction signing.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-23T08:02:43.167Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1724
File: web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx:86-89
Timestamp: 2024-10-23T08:02:43.167Z
Learning: In the `submitEvidence` function within `SubmitEvidenceModal.tsx`, most errors are caught by `wrapWithToast`, and the `catch` block is mainly for edge cases.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
🧬 Code graph analysis (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (5)
web-devtools/src/styles/Theme.tsx (1)
theme(3-10)web/src/pages/Home/TopJurors/index.tsx (1)
StyledLabel(47-49)web/src/hooks/useSpamEvidence.ts (1)
useSpamEvidence(33-43)web/src/hooks/queries/usePopulatedDisputeData.ts (1)
usePopulatedDisputeData(30-84)web/src/components/Divider.tsx (1)
Divider(3-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Analyze (javascript)
- GitHub Check: hardhat-tests
- GitHub Check: Mend Security Check
🔇 Additional comments (3)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (3)
6-6: LGTM! Clean import additions.The new imports are properly utilized throughout the component for type safety, data fetching, conditional rendering, and UI enhancement.
Also applies to: 15-15, 17-17, 24-24
41-77: LGTM! Well-structured styled components with proper visual hierarchy.The container and heading components create clear visual separation between the "Justifications" section and regular "Evidence" section. The margin handling properly spaces the sections with the divider.
117-120: LGTM! Proper TypeScript typing.The interface correctly defines the optional
arbitrableprop with the appropriateAddresstype from viem, maintaining backward compatibility.
Updated the evidence documentation link in EvidenceSchema.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts (1)
60-70: Well-structured evidence schema with good documentation.The schema appropriately references the evidence format specification and reuses existing validators for consistency. The distinction between required fields and optional "court UI specific" fields is clear.
📅 Optional: Consider adding timestamp range validation
If timestamp represents Unix time, you could add validation to ensure it's a positive number and within a reasonable range:
- timestamp: z.number().optional(), + timestamp: z.number().positive().optional(),This would prevent negative or zero timestamps, though it's not critical for the current implementation.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: hardhat-tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
kleros-sdk/src/dataMappings/utils/disputeDetailsSchema.ts (2)
26-28: LGTM! Transaction hash validation is correct.The schema correctly validates Ethereum transaction hashes by ensuring they are 66 characters long (0x prefix + 64 hex characters for 32 bytes) and match the hex format.
91-91: LGTM! Good backward compatibility with default empty array.The
extraEvidencesfield is well-integrated into the schema with a sensible default that ensures existing code won't break when the schema is updated.
jaybuidl
left a comment
jaybuidl
left a comment
There was a problem hiding this comment.
Published @kleros/[email protected]
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (1)
177-177: Empty state message may be misleading when arbitrable evidences exist.If
arbitrableEvidencesare rendered butdata.evidences.length === 0, the message "There is no evidence submitted yet" will still display below the arbitrable evidence cards. This might confuse users who see evidence cards above the message.Consider adjusting the condition or the message text:
💡 Possible fix
- {data && data.evidences.length === 0 ? <StyledLabel>There is no evidence submitted yet</StyledLabel> : null} + {data && data.evidences.length === 0 && isUndefined(arbitrableEvidences) ? ( + <StyledLabel>There is no evidence submitted yet</StyledLabel> + ) : null}Alternatively, update the message to clarify it refers to user-submitted evidence if you want to keep it visible.
🧹 Nitpick comments (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (1)
127-137: Consider using a more stable key than array index.Using
indexas the React key works for static lists but can cause issues if the list is reordered or items are added/removed. IftransactionHashis available and unique, it would be a better key candidate.Also, confirm that passing
evidence=""is intentional for arbitrable evidences, as it differs from theevidenceprop used in real evidences below.♻️ Suggested improvement for stable keys
- {!isUndefined(arbitrableEvidences) && arbitrableEvidences.length > 0 ? ( + {!isUndefined(arbitrableEvidences) && arbitrableEvidences.length > 0 ? ( <> - {arbitrableEvidences.map(({ name, description, fileURI, sender, timestamp, transactionHash }, index) => ( + {arbitrableEvidences.map(({ name, description, fileURI, sender, timestamp, transactionHash }, index) => ( <EvidenceCard - key={index} + key={transactionHash ?? `arbitrable-evidence-${index}`} evidence="" {...{ sender, timestamp, transactionHash, name, description, fileURI }} />
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
📚 Learning: 2024-10-14T15:31:27.821Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/pages/Cases/CaseDetails/Evidence/index.tsx:79-79
Timestamp: 2024-10-14T15:31:27.821Z
Learning: In the `EvidenceSearch` component, an undefined `evidenceGroup` prop is expected and handled within the function.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-14T13:58:25.708Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-12-06T13:04:50.495Z
Learnt from: kemuru
Repo: kleros/kleros-v2 PR: 1774
File: web/src/components/CasesDisplay/index.tsx:61-61
Timestamp: 2024-12-06T13:04:50.495Z
Learning: In `web/src/components/CasesDisplay/index.tsx`, the variables `numberDisputes` and `numberClosedDisputes` can sometimes be `NaN`, and should default to `0` using logical OR (`||`) to prevent display issues in the `StatsAndFilters` component.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-29T10:14:52.512Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1729
File: web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx:125-127
Timestamp: 2024-10-29T10:14:52.512Z
Learning: In `web/src/layout/Header/navbar/Menu/Settings/Notifications/FormContactDetails/index.tsx`, when `isEmailUpdateable` is false, `user?.emailUpdateableAt` is always defined. Therefore, using the non-null assertion `!` with `user?.emailUpdateableAt!` is acceptable because TypeScript may not infer its definiteness.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-05-09T13:39:15.086Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Parameters/NotablePersons/PersonFields.tsx:64-0
Timestamp: 2025-05-09T13:39:15.086Z
Learning: In PersonFields.tsx, the useEffect hook for address validation intentionally uses an empty dependency array to run only once on component mount. This is specifically designed for the dispute duplication flow when aliasesArray is already populated with addresses that need initial validation.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-05-15T06:50:40.859Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2025-09-09T13:33:46.896Z
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 2117
File: web/src/components/DisputeFeatures/Features/GatedErc1155.tsx:51-66
Timestamp: 2025-09-09T13:33:46.896Z
Learning: The `setDisputeData` function in `NewDisputeContext` at `web/src/context/NewDisputeContext.tsx` has signature `(disputeData: IDisputeData) => void` and only accepts direct state values, not functional updates like standard React state setters. It cannot be used with the pattern `setDisputeData((prev) => ...)`.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-09T10:22:41.474Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90
Timestamp: 2024-10-09T10:22:41.474Z
Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
📚 Learning: 2024-10-09T10:23:39.325Z
Learnt from: jaybuidl
Repo: kleros/kleros-v2 PR: 1582
File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:98-103
Timestamp: 2024-10-09T10:23:39.325Z
Learning: In `SelectArbitrable.tsx` of the web-devtools project (React/TypeScript), direct DOM manipulation using `child.click()` is acceptable when considered reasonably safe.
Applied to files:
web/src/pages/Cases/CaseDetails/Evidence/index.tsx
🧬 Code graph analysis (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (2)
web/src/hooks/useSpamEvidence.ts (1)
useSpamEvidence(33-43)web/src/hooks/queries/usePopulatedDisputeData.ts (1)
usePopulatedDisputeData(30-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: hardhat-tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (1)
web/src/pages/Cases/CaseDetails/Evidence/index.tsx (1)
84-95: Pass thearbitrableprop to theEvidencecomponent from the parent.The
CaseDetailsparent hasarbitrableavailable (dispute?.arbitrated.id) but doesn't pass it to<Evidence />, while other sibling routes like<Voting>do receive it. TheEvidencecomponent expects this prop to populate the arbitrable address in the dispute context data:web/src/pages/Cases/CaseDetails/index.tsx:81 - <Route path="evidence" element={<Evidence />} /> + <Route path="evidence" element={<Evidence {...{ arbitrable }} />} />⛔ Skipped due to learnings
Learnt from: tractorss Repo: kleros/kleros-v2 PR: 1982 File: web/src/pages/Resolver/Landing/index.tsx:62-62 Timestamp: 2025-05-15T06:50:40.859Z Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.Learnt from: Harman-singh-waraich Repo: kleros/kleros-v2 PR: 1703 File: web/src/pages/Cases/CaseDetails/Evidence/index.tsx:79-79 Timestamp: 2024-10-14T15:31:27.821Z Learning: In the `EvidenceSearch` component, an undefined `evidenceGroup` prop is expected and handled within the function.Learnt from: Harman-singh-waraich Repo: kleros/kleros-v2 PR: 1703 File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61 Timestamp: 2024-10-14T13:58:25.708Z Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.Learnt from: tractorss Repo: kleros/kleros-v2 PR: 2117 File: web/src/components/DisputeFeatures/Features/GatedErc1155.tsx:51-66 Timestamp: 2025-09-09T13:33:46.896Z Learning: The `setDisputeData` function in `NewDisputeContext` at `web/src/context/NewDisputeContext.tsx` has signature `(disputeData: IDisputeData) => void` and only accepts direct state values, not functional updates like standard React state setters. It cannot be used with the pattern `setDisputeData((prev) => ...)`.Learnt from: Harman-singh-waraich Repo: kleros/kleros-v2 PR: 1716 File: web-devtools/src/app/(main)/dispute-template/CustomContextInputs.tsx:29-42 Timestamp: 2024-10-28T05:55:12.728Z Learning: In the `CustomContextInputs` component located at `web-devtools/src/app/(main)/dispute-template/CustomContextInputs.tsx`, the `DisputeRequestParams` array is used to exclude certain variables from the custom input since they are already provided in a preceding component. Therefore, converting it to a type is unnecessary.Learnt from: kemuru Repo: kleros/kleros-v2 PR: 1774 File: web/src/components/CasesDisplay/index.tsx:61-61 Timestamp: 2024-12-06T13:04:50.495Z Learning: In `web/src/components/CasesDisplay/index.tsx`, the variables `numberDisputes` and `numberClosedDisputes` can sometimes be `NaN`, and should default to `0` using logical OR (`||`) to prevent display issues in the `StatsAndFilters` component.Learnt from: jaybuidl Repo: kleros/kleros-v2 PR: 1582 File: web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx:88-90 Timestamp: 2024-10-09T10:22:41.474Z Learning: Next.js recommends using the `useEffect` hook to set `isClient` and using `suppressHydrationWarning` as a workaround for handling hydration inconsistencies, especially when dealing with data like `knownArbitrables` that may differ between server-side and client-side rendering. This approach is acceptable in TypeScript/React applications, such as in `web-devtools/src/app/(main)/ruler/SelectArbitrable.tsx`.Learnt from: Harman-singh-waraich Repo: kleros/kleros-v2 PR: 1687 File: web/src/pages/Cases/CaseDetails/Evidence/SubmitEvidenceModal.tsx:67-84 Timestamp: 2024-09-12T14:45:48.019Z Learning: In the `submitEvidence` function of the SubmitEvidenceModal component, the try-catch block is specifically designed to handle errors from the `constructEvidence` function, which may throw an Error if file upload fails. This ensures that the file is successfully uploaded before proceeding with evidence submission and transaction signing.
tractorss
dismissed stale reviews from jaybuidl and coderabbitai[bot]
via
f7eb2df
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/src/components/EvidenceCard.tsx (1)
206-208: Guard condition checks the wrong variable.
transactionExplorerLinkis computed viauseMemowithtransactionHash ?? "", so it will always be a defined string (neverundefined). The checkisUndefined(transactionExplorerLink)on line 234 will always be false, potentially rendering a broken explorer link whentransactionHashis missing.Check
transactionHashdirectly instead:Suggested fix
const transactionExplorerLink = useMemo(() => { - return getTxnExplorerLink(transactionHash ?? ""); + return transactionHash ? getTxnExplorerLink(transactionHash) : undefined; }, [transactionHash]);Or alternatively, update the condition:
- {isUndefined(timestamp) || isUndefined(transactionExplorerLink) ? null : ( + {isUndefined(timestamp) || isUndefined(transactionHash) ? null : (Also applies to: 234-238
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
web/src/components/EvidenceCard.tsx
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/hooks/queries/usePopulatedDisputeData.ts:58-61
Timestamp: 2024-10-14T13:58:25.708Z
Learning: In `web/src/hooks/queries/usePopulatedDisputeData.ts`, the query and subsequent logic only execute when `disputeData.dispute?.arbitrableChainId` and `disputeData.dispute?.externalDisputeId` are defined, so `initialContext` properties based on these values are safe to use without additional null checks.
Learnt from: tractorss
Repo: kleros/kleros-v2 PR: 1982
File: web/src/pages/Resolver/Landing/index.tsx:62-62
Timestamp: 2025-05-15T06:50:40.859Z
Learning: In the Landing component, it's safe to pass `dispute?.dispute?.arbitrated.id as 0x${string}` to `usePopulatedDisputeData` without additional null checks because the hook internally handles undefined parameters through its `isEnabled` flag and won't execute the query unless all required data is available.
📚 Learning: 2024-10-14T15:31:27.821Z
Learnt from: Harman-singh-waraich
Repo: kleros/kleros-v2 PR: 1703
File: web/src/pages/Cases/CaseDetails/Evidence/index.tsx:79-79
Timestamp: 2024-10-14T15:31:27.821Z
Learning: In the `EvidenceSearch` component, an undefined `evidenceGroup` prop is expected and handled within the function.
Applied to files:
web/src/components/EvidenceCard.tsx
🧬 Code graph analysis (1)
web/src/components/EvidenceCard.tsx (2)
web/src/utils/index.ts (1)
isUndefined(5-6)web/src/utils/date.ts (1)
formatDate(23-37)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: Redirect rules - kleros-v2-neo
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Redirect rules - kleros-v2-testnet-devtools
- GitHub Check: Redirect rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-neo
- GitHub Check: Header rules - kleros-v2-testnet-devtools
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Header rules - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-neo
- GitHub Check: Pages changed - kleros-v2-testnet-devtools
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Pages changed - kleros-v2-testnet
- GitHub Check: Mend Security Check
- GitHub Check: Analyze (javascript)
- GitHub Check: hardhat-tests
🔇 Additional comments (2)
web/src/components/EvidenceCard.tsx (2)
187-191: LGTM!Making
sender,index, andtransactionHashoptional properly supports the newextraEvidencesfeature where these fields may not always be present.
213-233: LGTM!Conditional rendering for
indexandsenderis correctly implemented usingisUndefinedguards, ensuring the component gracefully handles missing optional data.
PR-Codex overview
This PR focuses on updating the
@kleros/kleros-sdkpackage to version2.4.0, adding new features and making adjustments to existing schemas and components.Detailed summary
package.jsonto2.4.0.extraEvidencesfield inDisputeDetailsSchema.EvidenceSchemafor evidence details.IEvidenceCardinterface to make fields optional.EvidenceCardcomponent to handle optional fields.disputeDetailsSchema.test.ts.Evidencecomponent to incorporatearbitrableEvidences.Summary by CodeRabbit
New Features
Bug Fixes / Robustness
Tests
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.