Conversation
❌ Deploy Preview for kleros-v2-testnet-devtools failed. Why did it fail? →
|
❌ 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. |
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThe pull request refactors dispute kit identification and configuration across the codebase by moving the Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
web/src/pages/Cases/CaseDetails/index.tsx (1)
3-3:⚠️ Potential issue | 🔴 CriticalRemove duplicate import of
Address.The file imports
Addressfrom"viem"twice: once as a runtime import (line 3) and once as a type-only import (line 7). This creates a duplicate identifier and will cause a TypeScript compilation error. SinceAddressis only used as a type (line 62), keep only the type-only import.🐛 Proposed fix to remove the duplicate import
-import { Address } from "viem"; - import { useTranslation } from "react-i18next"; import { Route, Routes, useParams, Navigate } from "react-router-dom"; import type { Address } from "viem";Also applies to: 7-7
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Cases/CaseDetails/index.tsx` at line 3, There is a duplicate import of Address—remove the runtime import import { Address } from "viem"; and keep the type-only import (e.g., import type { Address } from "viem";) since Address is used only as a type (see its usage in the CaseDetails component/props on line referencing the Address type). Update the imports so only the type-only import remains to avoid the duplicate identifier and TypeScript compile error.web/src/pages/Cases/CaseDetails/Appeal/Classic/index.tsx (1)
25-26:⚠️ Potential issue | 🟠 MajorRemove the
as NumberStringassertion or widen the type toNumberString | "".
amountstarts as"". After the 500ms debounce,useParsedAmountwill callparseUnits("", 18), which throws because empty strings are not valid numeric values. The disabled button doesn't prevent this render-time error. Either useNumberString | ""as the Fund prop type, or guard the empty case before passing touseParsedAmount.Also applies to: 51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Cases/CaseDetails/Appeal/Classic/index.tsx` around lines 25 - 26, The component initializes amount as "" but later casts it to NumberString when passed into useParsedAmount, causing parseUnits("", 18) to throw; update the Fund prop/type usage or guard the empty string: either widen the type to NumberString | "" for the Fund prop or check amount before calling useParsedAmount (e.g., short-circuit when amount === "") so parseUnits is never invoked with an empty string; adjust references in this file to amount, setAmount and the useParsedAmount call (and any prop expecting NumberString) accordingly and apply the same fix at the other occurrence around line 51.
🧹 Nitpick comments (2)
web/src/context/NewDisputeContext.tsx (1)
104-104: TightendisputeKitIdtyping to the enum for stronger safety.Now that initialization uses
DisputeKits, consider narrowingIDisputeData.disputeKitIdfromnumbertoDisputeKitsto avoid accidental invalid IDs.Suggested type-tightening diff
interface IDisputeData extends IDisputeTemplate { courtId?: string; numberOfJurors: number; arbitrationCost?: string; aliasesArray?: AliasArray[]; - disputeKitId?: number; + disputeKitId?: DisputeKits; disputeKitData?: IDisputeKitData; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/context/NewDisputeContext.tsx` at line 104, The IDisputeData.disputeKitId should be narrowed from number to the DisputeKits enum to enforce valid IDs: update the IDisputeData type declaration to use disputeKitId: DisputeKits, adjust the initial state in NewDisputeContext (where disputeKitId: DisputeKits.Classic is set) and update any places that construct or read IDisputeData to use DisputeKits values (or cast/convert inputs) so callers no longer rely on raw numbers; ensure imports reference the DisputeKits enum and run type checks to correct any mismatches.web/src/pages/Cases/CaseDetails/Voting/index.tsx (1)
81-84: Address the TODO: Consider a loading/error state instead of empty fragment.Returning an empty fragment when
disputeKitInfois undefined may cause layout shifts or confusing UX. Consider rendering a loading skeleton or an error message for unrecognized dispute kits.Would you like me to suggest a loading state implementation, or should this be tracked as a separate issue?
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/pages/Cases/CaseDetails/Voting/index.tsx` around lines 81 - 84, Replace the empty fragment return for the undefined disputeKitInfo with an explicit loading or error UI: instead of "if (!disputeKitInfo) return <>;</>", render a small LoadingSkeleton or a clear error/placeholder message (e.g., "Loading dispute kit…" or "Unknown dispute kit") and ensure the branch distinguishes loading vs unrecoverable error; update the component that reads disputeKitInfo and the subsequent VotingComponent usage so consumers see a stable placeholder while disputeKitInfo resolves and an error state when it truly cannot be loaded.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/actions/commit/params.ts`:
- Line 1: The code imports DisputeKits from the dispute-kits barrel; change
those imports to pull DisputeKits from the leaf disputeKits module (i.e. import
the specific disputeKits module rather than the barrel) so the registry isn't
loaded transitively; update the import in params.ts (and all other action-layer
files that import DisputeKits) to reference the leaf module instead of the
barrel.
In `@web/src/dispute-kits/disputeFeature.ts`:
- Around line 55-56: The TODO in disputeFeature.ts about moving disputekit data
and the 'type' handling to a dk config registry must be resolved: either
implement the registry now (create a DisputeKitConfigRegistry module, move the
disputekit structure and 'type' ownership into
registerDisputeKit/getDisputeKitConfig APIs and update disputeFeature.ts to read
from that registry instead of an inline TODO), or replace the TODO with a clear
linked issue reference (e.g. GH/JIRA ID) that describes the migration,
acceptance criteria, and owner; update the comment to include that issue link
and close the TODO to eliminate open static-analysis debt.
In `@web/src/dispute-kits/registry.ts`:
- Around line 106-107: getDisputeKitConfigByKitId currently calls
BigInt(disputeKitId) directly which will throw for non-integer strings; add
defensive validation so invalid inputs return undefined instead of throwing.
Update getDisputeKitConfigByKitId to: if disputeKitId is a number or bigint,
coerce to BigInt; if it is a string, validate it matches an integer pattern
(e.g. /^\d+$/) before converting or alternatively wrap BigInt(disputeKitId) in a
try/catch and return undefined on error; then use the validated BigInt value to
compare against BigInt(kit.id) while keeping DISPUTE_KIT_REGISTRY lookup logic
unchanged.
In `@web/src/hooks/useDisputeKitInfo.ts`:
- Around line 18-28: The useMemo for disputeKit unnecessarily lists props in its
dependency array causing re-renders; update the disputeKit useMemo (the function
that calls getDisputeKitConfigByAddress and getDisputeKitConfigByKitId) to
remove props from the dependency array and only include disputeKitAddress and
disputeKitId so the memo only recomputes when those specific values change.
In `@web/src/hooks/useVotingContext.tsx`:
- Around line 47-53: The read is being enabled with a fallback to
disputeKitClassicAbi even when useDisputeKitInfo returned undefined, causing
queries against unknown kits; update the useReadContract call in
useVotingContext (referencing useReadContract, disputeKitInfo,
disputeKitClassicAbi, disputeKitAddress, and functionName "isVoteActive") so the
query is only enabled when disputeKitInfo exists and provides a disputeKitAbi
(i.e., include disputeKitInfo != null && disputeKitInfo.disputeKitAbi in the
query.enabled condition) and only pass disputeKitInfo.disputeKitAbi to the abi
parameter instead of falling back to disputeKitClassicAbi.
In `@web/src/pages/Cases/CaseDetails/Appeal/index.tsx`:
- Around line 53-67: The current early return shows "Unable to load dispute kit"
before the dispute query resolves because disputeKitAddress is async; change the
guard so you only show the unsupported-kit state when you actually requested a
kit and it failed to resolve: remove the top-level if
(isUndefined(disputeKitInfo)) return ... and instead, inside the Periods.appeal
=== currentPeriodIndex branch before rendering <AppealComponent>, check that
disputeKitAddress (from disputeData?.dispute?.currentRound?.disputeKit?.address)
is defined and disputeKitInfo is undefined and only then render the "Unable to
load dispute kit" fallback; otherwise, when not in appeal period render
AppealHistory as before and allow useDisputeKitInfo (the hook) to resolve
without short-circuiting the entire component.
In `@web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx`:
- Around line 45-47: This component uses castCommit with a union-typed
disputeKitId causing a suppressed type error; narrow the commit payload so
TypeScript knows it's the Classic variant by ensuring the disputeKitId is typed
as DisputeKits.Classic (e.g., update the Commit component/prop type or the local
ICommit variable to ICommit & { disputeKitId: DisputeKits.Classic } or use the
ClassicCommitParams variant), then remove the // `@ts-expect-error` and let
castCommit accept the correctly discriminated Classic params; references:
castCommit, ICommit, DisputeKits, ClassicCommitParams, Commit (Commit.tsx).
---
Outside diff comments:
In `@web/src/pages/Cases/CaseDetails/Appeal/Classic/index.tsx`:
- Around line 25-26: The component initializes amount as "" but later casts it
to NumberString when passed into useParsedAmount, causing parseUnits("", 18) to
throw; update the Fund prop/type usage or guard the empty string: either widen
the type to NumberString | "" for the Fund prop or check amount before calling
useParsedAmount (e.g., short-circuit when amount === "") so parseUnits is never
invoked with an empty string; adjust references in this file to amount,
setAmount and the useParsedAmount call (and any prop expecting NumberString)
accordingly and apply the same fix at the other occurrence around line 51.
In `@web/src/pages/Cases/CaseDetails/index.tsx`:
- Line 3: There is a duplicate import of Address—remove the runtime import
import { Address } from "viem"; and keep the type-only import (e.g., import type
{ Address } from "viem";) since Address is used only as a type (see its usage in
the CaseDetails component/props on line referencing the Address type). Update
the imports so only the type-only import remains to avoid the duplicate
identifier and TypeScript compile error.
---
Nitpick comments:
In `@web/src/context/NewDisputeContext.tsx`:
- Line 104: The IDisputeData.disputeKitId should be narrowed from number to the
DisputeKits enum to enforce valid IDs: update the IDisputeData type declaration
to use disputeKitId: DisputeKits, adjust the initial state in NewDisputeContext
(where disputeKitId: DisputeKits.Classic is set) and update any places that
construct or read IDisputeData to use DisputeKits values (or cast/convert
inputs) so callers no longer rely on raw numbers; ensure imports reference the
DisputeKits enum and run type checks to correct any mismatches.
In `@web/src/pages/Cases/CaseDetails/Voting/index.tsx`:
- Around line 81-84: Replace the empty fragment return for the undefined
disputeKitInfo with an explicit loading or error UI: instead of "if
(!disputeKitInfo) return <>;</>", render a small LoadingSkeleton or a clear
error/placeholder message (e.g., "Loading dispute kit…" or "Unknown dispute
kit") and ensure the branch distinguishes loading vs unrecoverable error; update
the component that reads disputeKitInfo and the subsequent VotingComponent usage
so consumers see a stable placeholder while disputeKitInfo resolves and an error
state when it truly cannot be loaded.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6750d44f-1c9e-47de-8282-c05b56762643
📒 Files selected for processing (62)
web/src/actions/commit/builders/classic.builder.test.tsweb/src/actions/commit/builders/gated.builder.test.tsweb/src/actions/commit/builders/index.tsweb/src/actions/commit/builders/shutter.builder.test.tsweb/src/actions/commit/execute.tsweb/src/actions/commit/params.tsweb/src/actions/fundAppeal/builders/classic.builder.test.tsweb/src/actions/fundAppeal/builders/index.tsweb/src/actions/fundAppeal/execute.tsweb/src/actions/fundAppeal/params.tsweb/src/actions/reveal/builders/classic.builder.test.tsweb/src/actions/reveal/builders/gated.builder.test.tsweb/src/actions/reveal/builders/index.tsweb/src/actions/reveal/builders/shutter.builder.test.tsweb/src/actions/reveal/execute.tsweb/src/actions/reveal/params.tsweb/src/actions/reveal/resolveRevealInputs.integration.test.tsweb/src/actions/reveal/resolveRevealInputs.test.tsweb/src/actions/vote/builders/classic.builder.test.tsweb/src/actions/vote/builders/index.tsweb/src/actions/vote/execute.tsweb/src/actions/vote/params.tsweb/src/components/DisputeFeatures/Features/ArgentinaConsumerProtection.tsxweb/src/components/DisputeFeatures/Features/ClassicVote.tsxweb/src/components/DisputeFeatures/Features/GatedErc1155.tsxweb/src/components/DisputeFeatures/Features/GatedErc20.tsxweb/src/components/DisputeFeatures/Features/UniversityVote.tsxweb/src/components/DisputeFeatures/Features/index.tsxweb/src/components/DisputeFeatures/GroupsUI.tsxweb/src/consts/index.tsweb/src/context/NewDisputeContext.tsxweb/src/dispute-kits/disputeFeature.tsweb/src/dispute-kits/disputeKits.tsweb/src/dispute-kits/index.tsweb/src/dispute-kits/registry.tsweb/src/dispute-kits/types.tsweb/src/hooks/useDisputeKitAddresses.tsweb/src/hooks/useDisputeKitInfo.tsweb/src/hooks/useGatedTokenInfo.tsweb/src/hooks/useVotingContext.tsxweb/src/pages/Cases/CaseDetails/Appeal/Classic/Fund.tsxweb/src/pages/Cases/CaseDetails/Appeal/Classic/index.tsxweb/src/pages/Cases/CaseDetails/Appeal/Shutter/Fund.tsxweb/src/pages/Cases/CaseDetails/Appeal/Shutter/index.tsxweb/src/pages/Cases/CaseDetails/Appeal/index.tsxweb/src/pages/Cases/CaseDetails/MaintenanceButtons/DrawButton.tsxweb/src/pages/Cases/CaseDetails/Overview/DisputeKitOverviewExtraInfo/gated.tsxweb/src/pages/Cases/CaseDetails/Overview/index.tsxweb/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsxweb/src/pages/Cases/CaseDetails/Voting/Classic/Reveal.tsxweb/src/pages/Cases/CaseDetails/Voting/Classic/Vote.tsxweb/src/pages/Cases/CaseDetails/Voting/Classic/index.tsxweb/src/pages/Cases/CaseDetails/Voting/Shutter/Commit.tsxweb/src/pages/Cases/CaseDetails/Voting/Shutter/Reveal.tsxweb/src/pages/Cases/CaseDetails/Voting/Shutter/index.tsxweb/src/pages/Cases/CaseDetails/Voting/index.tsxweb/src/pages/Cases/CaseDetails/index.tsxweb/src/pages/Resolver/Landing/index.tsxweb/src/pages/Resolver/NavigationButtons/SubmitBatchDisputesButton.tsxweb/src/pages/Resolver/NavigationButtons/SubmitDisputeButton.tsxweb/src/pages/Resolver/Parameters/Court/FeatureSelection/index.tsxweb/wagmi.config.ts
💤 Files with no reviewable changes (4)
- web/src/pages/Cases/CaseDetails/Appeal/Shutter/index.tsx
- web/src/pages/Cases/CaseDetails/Appeal/Shutter/Fund.tsx
- web/src/hooks/useDisputeKitAddresses.ts
- web/src/consts/index.ts
| @@ -1,40 +1,40 @@ | |||
| import { DisputeKits } from "src/consts"; | |||
| import { DisputeKits } from "src/dispute-kits"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "DisputeKits declaration:"
fd '^disputeKits\.ts$' web/src/dispute-kits --exec sed -n '1,120p' {}
echo
echo "dispute-kits barrel exports:"
fd '^index\.ts$' web/src/dispute-kits --exec sed -n '1,120p' {}
echo
echo "registry imports that make the barrel heavy:"
fd '^registry\.ts$' web/src/dispute-kits --exec sed -n '1,160p' {}
echo
echo "action-layer imports of the barrel:"
rg -n 'from "src/dispute-kits"' web/src/actionsRepository: kleros/kleros-v2
Length of output: 5542
Import DisputeKits from the leaf module, not the dispute-kits barrel.
The src/dispute-kits barrel includes an explicit warning against this pattern: its own comment states that imports should come directly from ./disputeKits "so the registry is not loaded before mocks." The registry loads UI components and contract dependencies that action-layer code should not transitively depend on. This affects more than the three params.ts files mentioned; update all action-layer imports of DisputeKits to import from src/dispute-kits/disputeKits instead.
Proposed change
-import { DisputeKits } from "src/dispute-kits";
+import { DisputeKits } from "src/dispute-kits/disputeKits";📝 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.
| import { DisputeKits } from "src/dispute-kits"; | |
| import { DisputeKits } from "src/dispute-kits/disputeKits"; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/actions/commit/params.ts` at line 1, The code imports DisputeKits
from the dispute-kits barrel; change those imports to pull DisputeKits from the
leaf disputeKits module (i.e. import the specific disputeKits module rather than
the barrel) so the registry isn't loaded transitively; update the import in
params.ts (and all other action-layer files that import DisputeKits) to
reference the leaf module instead of the barrel.
| // TODO: move to dk cofig registry, issue: cannot figure out the correct usage of 'type' yet, | ||
| // we need this to structure disputekit data for the dispute, not sure how to move this to config yet |
There was a problem hiding this comment.
Resolve the open TODO before merge (or track it with a linked issue).
Line 55 and Line 56 leave unresolved behavior around type/registry ownership, and this keeps static analysis debt open. Please either implement the move now or attach an issue reference in the comment so ownership is explicit.
If you want, I can draft a scoped follow-up issue with acceptance criteria for the registry migration.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 55-55: Complete the task associated to this "TODO" comment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/dispute-kits/disputeFeature.ts` around lines 55 - 56, The TODO in
disputeFeature.ts about moving disputekit data and the 'type' handling to a dk
config registry must be resolved: either implement the registry now (create a
DisputeKitConfigRegistry module, move the disputekit structure and 'type'
ownership into registerDisputeKit/getDisputeKitConfig APIs and update
disputeFeature.ts to read from that registry instead of an inline TODO), or
replace the TODO with a clear linked issue reference (e.g. GH/JIRA ID) that
describes the migration, acceptance criteria, and owner; update the comment to
include that issue link and close the TODO to eliminate open static-analysis
debt.
| export function getDisputeKitConfigByKitId(disputeKitId: number | string | bigint) { | ||
| return Object.values(DISPUTE_KIT_REGISTRY).find((kit) => BigInt(kit.id) === BigInt(disputeKitId)); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "JavaScript BigInt behaviour on representative string inputs:"
node - <<'NODE'
for (const value of ["1", "Classic", "1-2", "NaN"]) {
try {
console.log(JSON.stringify(value), "=>", BigInt(value).toString());
} catch (error) {
console.log(JSON.stringify(value), "=> throws", error.name);
}
}
NODE
echo
echo "Call sites of dispute-kit lookup helpers:"
rg -n '\b(getDisputeKitConfigByKitId|useDisputeKitInfo)\s*\('Repository: kleros/kleros-v2
Length of output: 1075
🏁 Script executed:
cat -n web/src/hooks/useDisputeKitInfo.ts | head -40Repository: kleros/kleros-v2
Length of output: 1374
🏁 Script executed:
rg "disputeKitId" --type-list | head -20Repository: kleros/kleros-v2
Length of output: 449
🏁 Script executed:
rg -n "useDisputeKitInfo\s*\(\s*\{\s*disputeKitId" web/Repository: kleros/kleros-v2
Length of output: 42
🏁 Script executed:
rg -n "getDisputeKitConfigByKitId\(" --context=3 | grep -v "export function"Repository: kleros/kleros-v2
Length of output: 807
🏁 Script executed:
rg "disputeKitId" web/src --type-listRepository: kleros/kleros-v2
Length of output: 449
🏁 Script executed:
rg "disputeKitId\s*:" web/src -A 2 -B 2Repository: kleros/kleros-v2
Length of output: 18307
🏁 Script executed:
fd "\.graphql$|gql\." web/src | head -10Repository: kleros/kleros-v2
Length of output: 42
Add error handling to getDisputeKitConfigByKitId for defensive string validation.
The function accepts string but BigInt(disputeKitId) throws on non-integer values instead of returning undefined. While the string parameter is currently unused in the codebase (all callers pass parsed numbers), the exported function signature permits it, so defensive error handling is warranted.
Proposed hardening
export function getDisputeKitConfigByKitId(disputeKitId: number | string | bigint) {
- return Object.values(DISPUTE_KIT_REGISTRY).find((kit) => BigInt(kit.id) === BigInt(disputeKitId));
+ try {
+ const normalizedId = BigInt(disputeKitId);
+ return Object.values(DISPUTE_KIT_REGISTRY).find((kit) => BigInt(kit.id) === normalizedId);
+ } catch {
+ return undefined;
+ }
}📝 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 getDisputeKitConfigByKitId(disputeKitId: number | string | bigint) { | |
| return Object.values(DISPUTE_KIT_REGISTRY).find((kit) => BigInt(kit.id) === BigInt(disputeKitId)); | |
| export function getDisputeKitConfigByKitId(disputeKitId: number | string | bigint) { | |
| try { | |
| const normalizedId = BigInt(disputeKitId); | |
| return Object.values(DISPUTE_KIT_REGISTRY).find((kit) => BigInt(kit.id) === normalizedId); | |
| } catch { | |
| return undefined; | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/dispute-kits/registry.ts` around lines 106 - 107,
getDisputeKitConfigByKitId currently calls BigInt(disputeKitId) directly which
will throw for non-integer strings; add defensive validation so invalid inputs
return undefined instead of throwing. Update getDisputeKitConfigByKitId to: if
disputeKitId is a number or bigint, coerce to BigInt; if it is a string,
validate it matches an integer pattern (e.g. /^\d+$/) before converting or
alternatively wrap BigInt(disputeKitId) in a try/catch and return undefined on
error; then use the validated BigInt value to compare against BigInt(kit.id)
while keeping DISPUTE_KIT_REGISTRY lookup logic unchanged.
| const disputeKit = useMemo(() => { | ||
| if ("disputeKitAddress" in props && !isUndefined(disputeKitAddress)) { | ||
| return getDisputeKitConfigByAddress(disputeKitAddress); | ||
| } | ||
|
|
||
| if ("disputeKitId" in props && !isUndefined(disputeKitId)) { | ||
| return getDisputeKitConfigByKitId(disputeKitId); | ||
| } | ||
|
|
||
| return undefined; | ||
| }, [props, disputeKitAddress, disputeKitId]); |
There was a problem hiding this comment.
Remove props from the dependency array to prevent unnecessary recomputation.
Including props in the useMemo dependency array causes recomputation on every render since props is a new object reference each time. The individual disputeKitAddress and disputeKitId dependencies are sufficient.
🔧 Proposed fix
const disputeKit = useMemo(() => {
if ("disputeKitAddress" in props && !isUndefined(disputeKitAddress)) {
return getDisputeKitConfigByAddress(disputeKitAddress);
}
if ("disputeKitId" in props && !isUndefined(disputeKitId)) {
return getDisputeKitConfigByKitId(disputeKitId);
}
return undefined;
- }, [props, disputeKitAddress, disputeKitId]);
+ }, [disputeKitAddress, disputeKitId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/useDisputeKitInfo.ts` around lines 18 - 28, The useMemo for
disputeKit unnecessarily lists props in its dependency array causing re-renders;
update the disputeKit useMemo (the function that calls
getDisputeKitConfigByAddress and getDisputeKitConfigByKitId) to remove props
from the dependency array and only include disputeKitAddress and disputeKitId so
the memo only recomputes when those specific values change.
| const voteResult = useReadContract({ | ||
| address: disputeKitAddress as Address | undefined, | ||
| abi: disputeKitInfo?.disputeKitAbi ?? disputeKitClassicAbi, | ||
| functionName: "isVoteActive", | ||
| args: [BigInt(id ?? 0), BigInt(roundId ?? 0), BigInt(voteId ?? 0)], | ||
| query: { | ||
| enabled: isEnabled && disputeKitName === DisputeKits.ClassicUniversity, | ||
| enabled: !isUndefined(disputeKitAddress) && !isUndefined(roundId) && !isUndefined(voteId) && !isUndefined(id), |
There was a problem hiding this comment.
Don't query unknown kits with the Classic ABI.
If useDisputeKitInfo misses this address, the read still gets enabled and polls isVoteActive against that contract using disputeKitClassicAbi. Unsupported or misconfigured kits should short-circuit here, otherwise this can surface false vote state or repeated read errors.
Proposed guard
+ const canReadVoteStatus =
+ !isUndefined(disputeKitInfo) &&
+ !isUndefined(disputeKitAddress) &&
+ !isUndefined(roundId) &&
+ !isUndefined(voteId) &&
+ !isUndefined(id);
+
const voteResult = useReadContract({
address: disputeKitAddress as Address | undefined,
abi: disputeKitInfo?.disputeKitAbi ?? disputeKitClassicAbi,
functionName: "isVoteActive",
args: [BigInt(id ?? 0), BigInt(roundId ?? 0), BigInt(voteId ?? 0)],
query: {
- enabled: !isUndefined(disputeKitAddress) && !isUndefined(roundId) && !isUndefined(voteId) && !isUndefined(id),
+ enabled: canReadVoteStatus,
refetchInterval: REFETCH_INTERVAL,
},
});📝 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.
| const voteResult = useReadContract({ | |
| address: disputeKitAddress as Address | undefined, | |
| abi: disputeKitInfo?.disputeKitAbi ?? disputeKitClassicAbi, | |
| functionName: "isVoteActive", | |
| args: [BigInt(id ?? 0), BigInt(roundId ?? 0), BigInt(voteId ?? 0)], | |
| query: { | |
| enabled: isEnabled && disputeKitName === DisputeKits.ClassicUniversity, | |
| enabled: !isUndefined(disputeKitAddress) && !isUndefined(roundId) && !isUndefined(voteId) && !isUndefined(id), | |
| const canReadVoteStatus = | |
| !isUndefined(disputeKitInfo) && | |
| !isUndefined(disputeKitAddress) && | |
| !isUndefined(roundId) && | |
| !isUndefined(voteId) && | |
| !isUndefined(id); | |
| const voteResult = useReadContract({ | |
| address: disputeKitAddress as Address | undefined, | |
| abi: disputeKitInfo?.disputeKitAbi ?? disputeKitClassicAbi, | |
| functionName: "isVoteActive", | |
| args: [BigInt(id ?? 0), BigInt(roundId ?? 0), BigInt(voteId ?? 0)], | |
| query: { | |
| enabled: canReadVoteStatus, | |
| refetchInterval: REFETCH_INTERVAL, | |
| }, | |
| }); |
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 48-48: This assertion is unnecessary since it does not change the type of the expression.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/hooks/useVotingContext.tsx` around lines 47 - 53, The read is being
enabled with a fallback to disputeKitClassicAbi even when useDisputeKitInfo
returned undefined, causing queries against unknown kits; update the
useReadContract call in useVotingContext (referencing useReadContract,
disputeKitInfo, disputeKitClassicAbi, disputeKitAddress, and functionName
"isVoteActive") so the query is only enabled when disputeKitInfo exists and
provides a disputeKitAbi (i.e., include disputeKitInfo != null &&
disputeKitInfo.disputeKitAbi in the query.enabled condition) and only pass
disputeKitInfo.disputeKitAbi to the abi parameter instead of falling back to
disputeKitClassicAbi.
| const disputeKitAddress = disputeData?.dispute?.currentRound?.disputeKit?.address; | ||
| const disputeKitInfo = useDisputeKitInfo({ disputeKitAddress }); | ||
| // TODO: return a proper message | ||
| if (isUndefined(disputeKitInfo)) return <>Unable to load dispute kit</>; | ||
|
|
||
| const AppealComponent = disputeKitInfo.AppealComponent; | ||
| return ( | ||
| <Container> | ||
| {Periods.appeal === currentPeriodIndex ? ( | ||
| <> | ||
| {isClassicDisputeKit && ( | ||
| <Classic | ||
| isAppealMiniGuideOpen={isAppealMiniGuideOpen} | ||
| toggleAppealMiniGuide={toggleAppealMiniGuide} | ||
| {...{ disputeKitName }} | ||
| /> | ||
| )} | ||
| {isShutterDisputeKit && ( | ||
| <Shutter | ||
| isAppealMiniGuideOpen={isAppealMiniGuideOpen} | ||
| toggleAppealMiniGuide={toggleAppealMiniGuide} | ||
| {...{ disputeKitName }} | ||
| /> | ||
| )} | ||
| <AppealComponent | ||
| isAppealMiniGuideOpen={isAppealMiniGuideOpen} | ||
| toggleAppealMiniGuide={toggleAppealMiniGuide} | ||
| disputeKitId={disputeKitInfo.id} | ||
| /> |
There was a problem hiding this comment.
Don't return the unsupported-kit state before the dispute query resolves.
Because disputeKitAddress comes from async data, this branch can show Unable to load dispute kit on the first paint. Since the return happens before the period switch, it also hides AppealHistory outside the appeal period until the lookup finishes.
💡 Suggested guard placement
const disputeKitAddress = disputeData?.dispute?.currentRound?.disputeKit?.address;
const disputeKitInfo = useDisputeKitInfo({ disputeKitAddress });
- // TODO: return a proper message
- if (isUndefined(disputeKitInfo)) return <>Unable to load dispute kit</>;
-
- const AppealComponent = disputeKitInfo.AppealComponent;
+ const AppealComponent = disputeKitInfo?.AppealComponent;
return (
<Container>
{Periods.appeal === currentPeriodIndex ? (
<>
- <AppealComponent
- isAppealMiniGuideOpen={isAppealMiniGuideOpen}
- toggleAppealMiniGuide={toggleAppealMiniGuide}
- disputeKitId={disputeKitInfo.id}
- />
+ {!isUndefined(AppealComponent) ? (
+ <AppealComponent
+ isAppealMiniGuideOpen={isAppealMiniGuideOpen}
+ toggleAppealMiniGuide={toggleAppealMiniGuide}
+ disputeKitId={disputeKitInfo.id}
+ />
+ ) : !isUndefined(disputeKitAddress) ? (
+ <>Unable to load dispute kit</>
+ ) : null}
</>
) : (
<AppealHistory isAppealMiniGuideOpen={isAppealMiniGuideOpen} toggleAppealMiniGuide={toggleAppealMiniGuide} />
)}🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 55-55: Complete the task associated to this "TODO" comment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/Cases/CaseDetails/Appeal/index.tsx` around lines 53 - 67, The
current early return shows "Unable to load dispute kit" before the dispute query
resolves because disputeKitAddress is async; change the guard so you only show
the unsupported-kit state when you actually requested a kit and it failed to
resolve: remove the top-level if (isUndefined(disputeKitInfo)) return ... and
instead, inside the Periods.appeal === currentPeriodIndex branch before
rendering <AppealComponent>, check that disputeKitAddress (from
disputeData?.dispute?.currentRound?.disputeKit?.address) is defined and
disputeKitInfo is undefined and only then render the "Unable to load dispute
kit" fallback; otherwise, when not in appeal period render AppealHistory as
before and allow useDisputeKitInfo (the hook) to resolve without
short-circuiting the entire component.
| // @ts-expect-error Typescript is unable to handle the union type here. TODO: fix | ||
| await castCommit({ | ||
| type: disputeKitName ?? DisputeKits.Classic, | ||
| disputeKitId, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Expect: the commit param union is narrower than the broad `DisputeKits` prop,
# which explains the current `@ts-expect-error`.
sed -n '1,260p' web/src/actions/commit/params.ts
echo '---'
sed -n '1,260p' web/src/hooks/useCastCommit.tsx
echo '---'
sed -n '1,260p' web/src/dispute-kits/types.ts
echo '---'
sed -n '1,220p' web/src/pages/Cases/CaseDetails/Voting/Classic/index.tsx
echo '---'
sed -n '1,220p' web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsxRepository: kleros/kleros-v2
Length of output: 8348
Narrow the disputeKitId prop to DisputeKits.Classic in this component.
The @ts-expect-error suppresses the discriminated-union check on the castCommit payload. Since this component is exclusively in the Classic voting flow, ICommit.disputeKitId should be typed as DisputeKits.Classic instead of the broad DisputeKits enum. This allows TypeScript to discriminate the correct CommitParams variant (ClassicCommitParams) and removes the need for the suppression.
Suggested change
interface ICommit {
arbitrable: Address;
voteIDs: string[];
setIsOpen: (val: boolean) => void;
- disputeKitId: DisputeKits;
+ disputeKitId: DisputeKits.Classic;
}Then remove the @ts-expect-error comment from line 45.
🧰 Tools
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 45-45: Complete the task associated to this "TODO" comment.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/pages/Cases/CaseDetails/Voting/Classic/Commit.tsx` around lines 45 -
47, This component uses castCommit with a union-typed disputeKitId causing a
suppressed type error; narrow the commit payload so TypeScript knows it's the
Classic variant by ensuring the disputeKitId is typed as DisputeKits.Classic
(e.g., update the Commit component/prop type or the local ICommit variable to
ICommit & { disputeKitId: DisputeKits.Classic } or use the ClassicCommitParams
variant), then remove the // `@ts-expect-error` and let castCommit accept the
correctly discriminated Classic params; references: castCommit, ICommit,
DisputeKits, ClassicCommitParams, Commit (Commit.tsx).
| export function getDisputeKitConfig(id: DisputeKits) { | ||
| return DISPUTE_KIT_REGISTRY[id]; | ||
| } |
There was a problem hiding this comment.
IIUC, this function provides a way to get a config where we are sure it won't be undefined. That said, it doesn't seem to be used anywhere. Should this function and getDisputeKitConfigByKitId be combined into one, which accepts all types as input? It will be slightly more expensive, but since this function isn't used, it should be the same.
Unless, of course, you're planning on using this function heavily later, in which case it might be useful to keep.
| * @dev Assumes the DisputeKit is Gated, since it will only be called through gated kit routes | ||
| */ | ||
| export function useGatedTokenInfo(disputeId?: string, disputeKitAddress?: Address, currentRoundIndex?: number) { | ||
| export function useGatedTokenInfo(disputeId: string, disputeKitAddress: Address, currentRoundIndex: number) { |
There was a problem hiding this comment.
disputeKitAddress is unused. Maybe remove?
| const { disputeKitName } = useDisputeKitAddresses({ disputeKitAddress }); | ||
| const isClassicDisputeKit = isClassicLikeDisputeKit(disputeKitName); | ||
| const isShutterDisputeKit = isShutterLikeDisputeKit(disputeKitName); | ||
| const disputeKitAddress = disputeData?.dispute?.currentRound?.disputeKit?.address; |
There was a problem hiding this comment.
Removing the ?? undefined from this line causes a type error on the next one. Any particular reason to remove this?
| import React, { useState } from "react"; | ||
| import styled, { css } from "styled-components"; | ||
|
|
||
| import { Address } from "viem"; |
| const TokenImg: React.FC = () => { | ||
| if (isERC721 && imageUri && !imgError) { | ||
| return ( | ||
| <ImageContainer> | ||
| <NFTImage src={imageUri} alt={nftName || displayName} onError={() => setImgError(true)} loading="lazy" /> | ||
| </ImageContainer> | ||
| ); | ||
| } else if (isERC721) { | ||
| return ( | ||
| <ImageContainer> | ||
| <PlaceholderText>🖼️</PlaceholderText> | ||
| </ImageContainer> | ||
| ); | ||
| } | ||
| return null; | ||
| }; |
There was a problem hiding this comment.
I normally don't define components inside other components to avoid unnecessary remounts.
| interface IOverview { | ||
| arbitrable?: Address; | ||
| courtID?: string; | ||
| currentPeriodIndex: number; |
There was a problem hiding this comment.
Not sure if it's because of your changes, but this is unused.
| parseInt(dispute?.dispute?.currentRoundIndex ?? "0", 10) | ||
| ); | ||
| const disputeKitAddress = dispute?.dispute?.currentRound.disputeKit?.address; | ||
| const currentRoundIndex = dispute?.dispute?.currentRoundIndex ?? 0n; |
There was a problem hiding this comment.
Seems to me we are not handling types properly here. Recently, I changed the subgraph to map BigInt to string, so this comes as a string and we give it a BigInt fallback. However, the DisputeKitOverviewExtraInfoComponent expects it to be a number, which comes from the types file. We might want to consolidate this, wdyt?
| @@ -43,10 +43,10 @@ const Vote: React.FC<IVote> = ({ arbitrable, voteIDs, setIsOpen, disputeKitName | |||
| choice: voteOption, | |||
| salt: BigInt(disputeData?.dispute?.currentRoundIndex), | |||
There was a problem hiding this comment.
Not done on this PR, but here this can be undefined and there's a type error. We could implement the same guard that's on Commit.tsx and Reveal.tsx.
Note that this guard approach is also valid for the other type issues I mentioned above, related to the disputeKitAddress.
| [currentPeriodIndex] | ||
| ); | ||
|
|
||
| const disputeKitAddress = disputeData?.dispute?.currentRound?.disputeKit?.address; |
|
|
||
| import { useTranslation } from "react-i18next"; | ||
| import { Route, Routes, useParams, Navigate } from "react-router-dom"; | ||
| import type { Address } from "viem"; |
There was a problem hiding this comment.
You probably meant to change the existing import :)
2 participants
Summary by CodeRabbit
Release Notes