Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds the Drop Forge feature (craft & launch) with pages, client components, hooks, API client/endpoints, Arweave upload workflows, multi-chain/testnet support, manifold minting refactor (chain-aware, new metadata types), UI primitives/icons, and extensive test updates and reorganizations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Browser
participant Client as DropForge<br/>Launch/Craft Client
participant API as Minting-Claims API
participant Storage as Arweave/Upload
participant Chain as Blockchain/Manifold
User->>Browser: Open Drop Forge page / perform upload or init
Browser->>Client: render with claimId & config (chain/contract)
Client->>API: GET /minting-claims/{id} (roots/proofs/metadata)
API-->>Client: MintingClaim + roots/proofs
User->>Client: Upload image/animation
Client->>API: POST /minting-claims/{id}/arweave-upload or multipart upload
API->>Storage: store file
Storage-->>API: location
API-->>Client: stored location
User->>Client: Initialize/update on-chain
Client->>API: GET proofs/roots by address or merkleRoot
API-->>Client: proofs
Client->>Chain: send tx (using chain id & contract)
Chain-->>Client: txHash / receipt
Client->>API: PATCH /minting-claims/{id} (update state)
API-->>Client: updated claim
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
components/navigation/NavItem.tsx (1)
58-68:⚠️ Potential issue | 🟠 MajorTwo correctness issues in the
useEffect: stale closures and wrong title on reset.1 — Incomplete dependency array (stale closure risk)
setTitleandremoveAllDeliveredNotificationsare missing from the dependency array. If either function changes identity between renders (especiallyremoveAllDeliveredNotifications, whose stability is unknown), the effect closes over a stale reference and silently does nothing or calls the wrong version.2 — Hardcoded
"6529.io"overrides the current-page title everywhereWhen
!haveUnreadNotifications,setTitle("6529.io")is called unconditionally. NavItem lives in a persistent navigation bar, so this fires on any page when unread count drops to zero—clobbering wave titles, profile titles, etc. It also produces the wrong title on staging (whereDEFAULT_TITLEis"6529 Staging").The else-branch should either be removed entirely (letting
TitleContextmanage the reset) or useDEFAULT_TITLE:+import { DEFAULT_TITLE, useTitle } from "@/contexts/TitleContext"; ... useEffect(() => { if (item.name !== "Notifications") return; - setTitle( - haveUnreadNotifications - ? `(${notifications?.unread_count}) Notifications | 6529.io` - : "6529.io" - ); + if (haveUnreadNotifications) { + setTitle(`(${notifications?.unread_count}) Notifications | ${DEFAULT_TITLE}`); + } if (!haveUnreadNotifications) { removeAllDeliveredNotifications(); } }, [ haveUnreadNotifications, notifications?.unread_count, item.name, setTitle, removeAllDeliveredNotifications, ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/navigation/NavItem.tsx` around lines 58 - 68, The effect in NavItem (useEffect watching haveUnreadNotifications and notifications?.unread_count) is missing setTitle and removeAllDeliveredNotifications from its dependency array and also unconditionally resets the title to the hardcoded "6529.io" which clobbers page-specific titles; update the dependency array to include setTitle and removeAllDeliveredNotifications to avoid stale closures, and change the branch that resets the title so it does not use the hardcoded string — either remove the reset entirely and let TitleContext manage it, or use the DEFAULT_TITLE constant (or call the TitleContext reset API) when !haveUnreadNotifications; keep the effect gated by item.name === "Notifications" as-is.openapi.yaml (1)
9792-9814:⚠️ Potential issue | 🟡 MinorConstrain wallet fields to Ethereum address format.
New wallet-typed fields currently accept arbitrary strings, weakening contract validation.
🛡️ Proposed validation constraints
ApiSeizeSettings: @@ distribution_admin_wallets: type: array items: type: string + pattern: ^0x[a-fA-F0-9]{40}$ claims_admin_wallets: type: array items: type: string + pattern: ^0x[a-fA-F0-9]{40}$ PhaseAirdrop: @@ wallet: type: string + pattern: ^0x[a-fA-F0-9]{40}$Also applies to: 11837-11847
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 9792 - 9814, The wallet array properties distribution_admin_wallets and claims_admin_wallets currently accept any string; update their item schema in openapi.yaml to validate Ethereum addresses by replacing the unconstrained string type with a patterned string (e.g., add pattern: "^0x[a-fA-F0-9]{40}$" and optionally format: "ethereum-address") for the items of both arrays (and apply the same change at the other occurrence noted), ensuring each array item must match the 0x-prefixed 40-hex-character Ethereum address format.components/providers/AppKitAdapterManager.ts (1)
147-170:⚠️ Potential issue | 🔴 CriticalCache key ignores
chains, risking stale adapter reuse with wrong network configuration.
getCacheKey(line 222) only incorporates wallet addresses into the cache key. WhencreateAdapterWithCacheis called with the same wallets but differentchains(e.g., switching from mainnet to testnet), the cache returns an adapter configured for the original chains instead of the requested ones.Include chain IDs in the cache key to ensure network configuration matches the request:
🐛 Proposed fix
- private getCacheKey(wallets: AppWallet[]): string { + private getCacheKey(wallets: AppWallet[], chains: Chain[]): string { if (!Array.isArray(wallets)) { throw new AdapterError( "ADAPTER_013: Cannot generate cache key: wallets must be an array" ); } const addresses = wallets.map((w) => { if (!w?.address) { throw new AdapterError( "ADAPTER_014: Cannot generate cache key: invalid wallet object" ); } return w.address; }); if (addresses.length === 0) { - return "empty-wallets"; + return `empty-wallets:${chains.map((c) => c.id).sort().join(",")}`; } const sortedAddresses = addresses.toSorted((a, b) => a.localeCompare(b)); - return sortedAddresses.join(","); + const chainIds = chains.map((c) => c.id).sort().join(","); + return `${sortedAddresses.join(",")}:${chainIds}`; }And update the call site at line 156:
- const cacheKey = this.getCacheKey(appWallets); + const cacheKey = this.getCacheKey(appWallets, chains);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/providers/AppKitAdapterManager.ts` around lines 147 - 170, The cache key currently ignores chains causing adapters to be reused across different network configs; update getCacheKey to accept chains (e.g., getCacheKey(appWallets, chains)) and incorporate deterministic chain identifiers (sorted chain IDs or chain.network/chain.id) into the key, then modify createAdapterWithCache to call this.getCacheKey(appWallets, chains) when looking up/setting adapterCache and when storing the newly created adapter returned by createAdapter; ensure any currentAdapter/currentWallets tracking is updated to also reflect the chains if you track them (e.g., currentChains) so the adapter returned always matches the requested chains.components/manifold-minting/ManifoldMintingWidget.tsx (2)
57-96:⚠️ Potential issue | 🟠 MajorGuard against stale merkle-proof fetch responses.
The async fetch can resolve out of order when
mintForAddresschanges, and an older response may overwrite newer proofs.🛠️ Proposed fix (effect-level stale guard)
useEffect(() => { + let active = true; mintWrite.reset(); setMintStatus(<></>); setMintError(""); @@ if (mintForAddress && props.claim.merkleRoot) { setIsError(false); setFetchingMerkle(true); getMemesMintingProofsByAddress( props.claim.instanceId, props.claim.merkleRoot, mintForAddress ) .then((response) => { + if (!active) return; const mappedProofs: MintingClaimsProofItem[] = ( response.proofs ?? [] ).map((proof) => ({ merkle_proof: proof.merkle_proof ?? [], value: Number(proof.value ?? 0), })); setMerkleProofs(mappedProofs); }) .catch(() => { + if (!active) return; setIsError(true); setMerkleProofs([]); }) .finally(() => { + if (!active) return; setFetchingMerkle(false); }); } + return () => { + active = false; + }; }, [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifold-minting/ManifoldMintingWidget.tsx` around lines 57 - 96, The fetch in the useEffect for merkle proofs can return out-of-order and overwrite newer data; add an effect-level stale guard: create a local "cancelled" or "currentRequestId" token inside the useEffect before calling getMemesMintingProofsByAddress, capture it in then/catch/finally handlers, and only call setMerkleProofs, setIsError, and setFetchingMerkle if the token still matches (i.e., request is not stale) — update the handlers around getMemesMintingProofsByAddress in the useEffect that references mintForAddress, props.claim.merkleRoot, props.claim.instanceId, and ensure to clean up by invalidating the token in the effect cleanup to prevent stale responses from applying.
100-108:⚠️ Potential issue | 🟠 MajorReplace side-effect-only
mapingetReadContractsParams.Line 100 uses
mapbut never returns from the callback; this violates Biome'suseIterableCallbackReturnrule and will keep lint red.🛠️ Proposed fix
function getReadContractsParams() { - const params: any = []; - merkleProofs.map((mp) => { - params.push({ + return merkleProofs.map((mp) => ({ address: MANIFOLD_LAZY_CLAIM_CONTRACT as `0x${string}`, abi: props.abi, chainId: props.chain.id, functionName: "checkMintIndex", args: [props.contract, props.claim.instanceId, mp.value], - }); - }); - return params; + })); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifold-minting/ManifoldMintingWidget.tsx` around lines 100 - 108, The current getReadContractsParams uses merkleProofs.map with a side-effect (pushing into params) which violates the useIterableCallbackReturn rule; change the iteration to a side-effect-appropriate loop such as merkleProofs.forEach or a for...of loop and push the same objects into params (using MANIFOLD_LAZY_CLAIM_CONTRACT, props.abi, props.chain.id, functionName "checkMintIndex", and args [props.contract, props.claim.instanceId, mp.value]) so the callback returns nothing and the lint rule is satisfied.components/waves/memes/traits/TextTrait.tsx (1)
71-79:⚠️ Potential issue | 🟡 Minor
handleBlurcomparison doesn't normalize to string, unlike the rest of the component.Line 72 compares
inputRef.current.value(always a string) directly againsttraits[field](which could be a number). The rest of the component normalizes via.toString()(Lines 36, 83, 91). Whentraits[field]is numeric, strict!==between"42"and42is alwaystrue, causing a spuriousupdateTextcall on every blur.Proposed fix
const handleBlur = useCallback(() => { - if (inputRef.current && inputRef.current.value !== traits[field]) { + if (inputRef.current && inputRef.current.value !== ((traits[field] as string) ?? "").toString()) { updateText(field, inputRef.current.value); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/traits/TextTrait.tsx` around lines 71 - 79, handleBlur currently compares inputRef.current.value (string) to traits[field] without normalizing, causing updates when traits[field] is numeric; change the comparison in handleBlur to compare inputRef.current.value to String(traits[field]) (or traits[field]?.toString()) before calling updateText, keeping the rest of the behavior (calling updateText(field, inputRef.current.value) and invoking onBlur(field) if present) and preserving the dependency array that includes field, traits, updateText, and onBlur.
🧹 Nitpick comments (35)
components/distribution-plan-tool/review-distribution-plan/table/ReviewDistributionPlanTableRow.tsx (1)
194-194: Use the shared count formatter instead of a local duplicate.There is already a project helper (
helpers/format.helpers.ts) for this. Reusing it keeps formatting behavior consistent and preserves defensive handling for invalid values.♻️ Proposed refactor
+import { formatCount } from "@/helpers/format.helpers"; ... - const formatCount = (count: number) => count.toLocaleString(); ... <td className={commonClasses}> - {isPublic ? "-" : formatCount(item.walletsCount)} + {isPublic ? "-" : (formatCount(item.walletsCount) ?? "-")} </td> <td className={commonClasses}> - {isPublic ? "-" : formatCount(item.spotsCount)} + {isPublic ? "-" : (formatCount(item.spotsCount) ?? "-")} </td>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/distribution-plan-tool/review-distribution-plan/table/ReviewDistributionPlanTableRow.tsx` at line 194, Replace the local const formatCount defined in ReviewDistributionPlanTableRow.tsx with the shared formatter exported from helpers/format.helpers.ts: remove the local function (formatCount) and import the shared formatter (formatCount) from "helpers/format.helpers.ts", then use that imported function wherever the local one was used so you retain the shared formatting logic and its defensive handling for invalid values.services/api/common-api.ts (1)
359-378:commonApiPatchlooks good; consider addingsignaltocommonApiPutfor consistency.
commonApiPatchcorrectly forwardssignaltoexecuteApiRequest, butcommonApiPut(line 340) has nosignalparameter. This inconsistency means PUT requests cannot be aborted, while the equivalent PATCH can.♻️ Suggested addition to
commonApiPutexport const commonApiPut = async <T, U, Z = Record<string, string>>(param: { endpoint: string; body: T; headers?: Record<string, string> | undefined; params?: Z | undefined; + signal?: AbortSignal | undefined; }): Promise<U> => { const url = buildUrl( param.endpoint, param.params as Record<string, string> | undefined ); return executeApiRequest<U>( url, "PUT", getHeaders(param.headers, true), - JSON.stringify(param.body) + JSON.stringify(param.body), + param.signal ); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/api/common-api.ts` around lines 359 - 378, commonApiPut is missing an AbortSignal parameter and doesn't forward it to executeApiRequest, making PUT requests non-cancellable; add an optional signal?: AbortSignal to the commonApiPut param type and pass param.signal into the executeApiRequest call (same as commonApiPatch), and ensure buildUrl/getHeaders usage remains unchanged so PUT behaves consistently with commonApiPatch.components/user/subscriptions/UserPageSubscriptionsTopUp.tsx (1)
48-56: SimplifygetTopUpModalEmoji— line 55 is dead code.All four union members are explicitly handled by lines 51–54, making the final
returnon line 55 unreachable. An object map eliminates the dead fallthrough and makes the mapping clearer at a glance.♻️ Proposed refactor
-function getTopUpModalEmoji( - status: "confirm_wallet" | "submitted" | "success" | "error" -): string { - if (status === "success") return "/emojis/sgt_saluting_face.webp"; - if (status === "error") return "/emojis/sgt_sob.webp"; - if (status === "confirm_wallet") return "/emojis/sgt_flushed.webp"; - if (status === "submitted") return "/emojis/sgt_flushed.webp"; - return "/emojis/sgt_flushed.webp"; -} +const TOP_UP_MODAL_EMOJIS = { + success: "/emojis/sgt_saluting_face.webp", + error: "/emojis/sgt_sob.webp", + confirm_wallet: "/emojis/sgt_flushed.webp", + submitted: "/emojis/sgt_flushed.webp", +} as const satisfies Record<"confirm_wallet" | "submitted" | "success" | "error", string>; + +function getTopUpModalEmoji( + status: keyof typeof TOP_UP_MODAL_EMOJIS +): string { + return TOP_UP_MODAL_EMOJIS[status]; +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/subscriptions/UserPageSubscriptionsTopUp.tsx` around lines 48 - 56, getTopUpModalEmoji currently uses multiple if statements with a dead fallback return; replace this with a simple object map keyed by the status union used in getTopUpModalEmoji to return the emoji string (e.g., map "confirm_wallet", "submitted", "success", "error" to their respective paths) and return map[status] (or a typed lookup with a default if needed) so the unreachable final return is removed and the mapping is clearer and exhaustive; update the function getTopUpModalEmoji accordingly.components/navigation/NavItem.tsx (1)
35-40:useWaveData+useWavefire for everyNavIteminstance, but the result is only used for the active state check.Every nav entry (Home, Waves, Messages, Notifications, …) independently calls
useWaveDataanduseWavewith the samewaveIdFromQuery, producing redundant hook subscriptions/queries per render. TheisCurrentWaveDmValueresult is only needed byisNavItemActive, which in turn is only meaningful for the Messages item.Consider lifting the wave-DM resolution out of
NavIteminto its parent (the nav container), passingisCurrentWaveDmas a prop, so the query runs exactly once.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/navigation/NavItem.tsx` around lines 35 - 40, NavItem currently calls useWaveData({ waveId: waveIdFromQuery, onWaveNotFound: () => {} }) and useWave(waveData) for every instance to compute isCurrentWaveDmValue only used in isNavItemActive — lift this logic to the nav container (parent) so the hook/query runs once: call useWaveData and useWave in the parent using waveIdFromQuery, derive isCurrentWaveDm, then remove useWaveData/useWave from NavItem and add a new prop (e.g. isCurrentWaveDm) to NavItem and use that inside isNavItemActive for the Messages item. Ensure you preserve the onWaveNotFound behavior in the parent and update all NavItem invocations to pass the new prop.components/header/AppUserConnect.tsx (1)
20-33: Extract shared chain-switch behavior into a reusable hook/helper.This chain resolution + switch logic is duplicated in
components/header/user/proxy/HeaderUserProxyDropdown.tsx(line range 40-78). Consolidating it will avoid behavior drift and keep fixes consistent.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/header/AppUserConnect.tsx` around lines 20 - 33, Extract the duplicated chain resolution and switch logic into a shared hook (e.g., useChainSwitcher) that uses useChains, useChainId, and useSwitchChain to expose currentChainName and a switchToNextChain function; replace the inline logic in AppUserConnect (currentChain, onSwitchChain) and the duplicate block in HeaderUserProxyDropdown.tsx with calls to useChainSwitcher to read currentChainName and invoke switchToNextChain, ensuring the new hook returns the same behavior (find next chain where c.id !== chainId and call switchChain with { chainId: next.id }) and export it for reuse.components/drops/view/item/content/media/MediaDisplayGLB.tsx (1)
158-161: Re-stroked edges visually thicken shared cube edges.The three
<path>elements at lines 159–161 redraw edges that are already fully stroked by the face paths above (top face, left face, right face each emitstroke="currentColor"). The overlap doubles the apparent stroke weight on every shared silhouette edge and the centre vertical, which may appear unintentionally bold—especially at small button sizes (tw-h-9 tw-w-9).Consider either removing the re-stroke group entirely, or converting the face paths to
fill-only (stroke="none") and relying solely on the re-stroke paths for edge rendering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/media/MediaDisplayGLB.tsx` around lines 158 - 161, The SVG in MediaDisplayGLB.tsx currently re-strokes the cube edges with three extra <path> elements (the paths drawing the silhouette and center vertical) which doubles stroke weight against the face paths; remove those three re-stroke <path> elements (the ones with d="M6 9v6l6 3 6-3v-6", d="M6 9l6-3 6 3", and d="M12 6v12") or alternatively change the face paths that currently include stroke="currentColor" to stroke="none" and keep only the re-stroke paths for edge rendering so shared edges render at a single intended weight (adjust whichever approach keeps visual parity at small sizes like tw-h-9 tw-w-9).openapi.yaml (1)
2428-2450: Use202 Acceptedfor queued Arweave uploads.The description says this operation enqueues async work;
202is a clearer status contract than200.♻️ Proposed response code adjustment
responses: - "200": - description: successful operation + "202": + description: Accepted - upload enqueued content: application/json: schema: $ref: "#/components/schemas/MintingClaim"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 2428 - 2450, The OpenAPI response for the async queue endpoint postMintingClaimArweaveUpload should return 202 Accepted instead of 200; update the responses block for operationId postMintingClaimArweaveUpload to use "202" with a description like "accepted; work queued" (and adjust any examples/schemas tied to the "200" response) so the contract clearly reflects that the upload is enqueued asynchronously.components/common/icons/DropForgeIcon.tsx (1)
6-11: Consider defaulting this SVG to decorative for a11yIf this icon is used next to visible text (e.g., nav labels), adding
aria-hiddenandfocusable="false"avoids redundant screen reader output.Proposed tweak
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 32 32" preserveAspectRatio="xMidYMid meet" className={className} + aria-hidden="true" + focusable="false" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/common/icons/DropForgeIcon.tsx` around lines 6 - 11, The SVG in the DropForgeIcon component is missing attributes to mark it decorative; update the <svg> element inside DropForgeIcon to include aria-hidden="true" and focusable="false" so screen readers skip it by default (if you need it to be accessible in some cases, add an optional prop like "ariaLabel" or "decorative" to conditionally render aria-hidden/role/aria-label instead). Target the DropForgeIcon component's <svg> element (the one with className={className}) and add those attributes or a small prop toggle to control them.contexts/SeizeSettingsContext.tsx (1)
51-52: Good defaults added — consider defensive merge on fetch tooTo avoid
undefinedarrays if the API payload is partial, normalize these fields duringsetSeizeSettingsas well.Proposed hardening
setSeizeSettings({ ...settings, memes_wave_id: publicEnv.DEV_MODE_MEMES_WAVE_ID ?? settings.memes_wave_id, curation_wave_id: publicEnv.DEV_MODE_CURATION_WAVE_ID ?? settings.curation_wave_id, + distribution_admin_wallets: settings.distribution_admin_wallets ?? [], + claims_admin_wallets: settings.claims_admin_wallets ?? [], });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@contexts/SeizeSettingsContext.tsx` around lines 51 - 52, When updating state from the API, ensure you defensive-merge/normalize missing array fields so they never become undefined: inside the setSeizeSettings updater (or reducer) coalesce distribution_admin_wallets and claims_admin_wallets to [] when the incoming payload omits them, e.g. merge the existing state with the fetched payload and default these two symbols (distribution_admin_wallets, claims_admin_wallets) to empty arrays before calling setSeizeSettings so the component always sees arrays.components/home/now-minting/NowMintingCountdown.tsx (1)
26-33: Unnecessary intermediate object and redundant conditional spread.The
countdownOptionsvariable is created only to be immediately spread intouseMintCountdownState. SincehideMintBtnis an optional parameter, passing it asundefinedis identical to omitting it, making the conditional spread needless.♻️ Suggested simplification
- const countdownOptions = { - contract, - chainId, - ...(hideMintBtn === undefined ? {} : { hideMintBtn }), - }; - const state = useMintCountdownState(nftId, { - ...countdownOptions, - }); + const state = useMintCountdownState(nftId, { contract, chainId, hideMintBtn });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/home/now-minting/NowMintingCountdown.tsx` around lines 26 - 33, The temporary countdownOptions object and the conditional spread are unnecessary; pass the props directly to useMintCountdownState instead. Replace the creation/use of countdownOptions by calling useMintCountdownState(nftId, { contract, chainId, hideMintBtn }) so hideMintBtn can be undefined (no special-case spread needed) and remove countdownOptions from the file; update references to countdownOptions accordingly to use the direct object literal in the useMintCountdownState call.components/drop-forge/DropForgeTestnetIndicator.tsx (1)
21-21: Trailing space in className whenclassNameis empty.When
classNamedefaults to"", the template literal produces a trailing space (e.g.,"...sm:tw-text-base "). This is harmless but unclean.♻️ Suggested fix
- className={`tw-inline-flex tw-items-center tw-rounded-md tw-border tw-border-amber-400/60 tw-bg-amber-500/15 tw-px-2.5 tw-py-1 tw-text-sm tw-font-semibold tw-text-amber-100 sm:tw-text-base ${className}`} + className={`tw-inline-flex tw-items-center tw-rounded-md tw-border tw-border-amber-400/60 tw-bg-amber-500/15 tw-px-2.5 tw-py-1 tw-text-sm tw-font-semibold tw-text-amber-100 sm:tw-text-base${className ? ` ${className}` : ""}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeTestnetIndicator.tsx` at line 21, The className string in the DropForgeTestnetIndicator component produces a trailing space when the prop/className variable is empty; update the className construction inside the JSX (the className attribute in DropForgeTestnetIndicator) to avoid the trailing space by conditionally appending the external className (e.g., use an array/filter/join pattern or a conditional like adding `' ' + className` only when className is truthy, or replace with a small utility like clsx) so the rendered class attribute has no extra trailing space when className === "".hooks/useIsDropForgeAdmin.ts (1)
28-28:readResult.isLoadingis redundant alongsidereadResult.isFetching.In TanStack Query v5 (used by wagmi v2),
isLoadingis derived asisPending && isFetching. This means wheneverisLoadingistrue,isFetchingis alsotrue. Therefore, the|| readResult.isLoadingbranch in the OR expression never adds a new condition—isFetchingalone covers all cases.♻️ Suggested simplification
- isFetching: Boolean(address) && (readResult.isLoading || readResult.isFetching), + isFetching: Boolean(address) && readResult.isFetching,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useIsDropForgeAdmin.ts` at line 28, In useIsDropForgeAdmin, simplify the computed isFetching flag by removing the redundant readResult.isLoading check: update the expression that sets isFetching (which currently references Boolean(address) && (readResult.isLoading || readResult.isFetching)) to only depend on readResult.isFetching together with the address check; keep the Boolean(address) AND readResult reference, but drop readResult.isLoading to avoid the redundant condition.components/drop-forge/DropForgeNoPower.tsx (1)
12-14: Usereplacefor forced redirects.On Line 13,
router.push("/")keeps the unauthorized page in history.router.replace("/")avoids back-button loops into the no-power screen.↩️ Suggested change
- const t = setTimeout(() => router.push("/"), 1000); + const t = setTimeout(() => router.replace("/"), 1000);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeNoPower.tsx` around lines 12 - 14, In DropForgeNoPower, the timeout handler currently uses router.push("/") which leaves the no-power page in history and can cause back-button loops; change the timeout callback in the seconds === 1 branch (where setTimeout is assigned to t) to call router.replace("/") instead of router.push("/") and keep the existing cleanup return () => clearTimeout(t) unchanged so the timeout is properly cleared if the component unmounts.components/common/icons/DropForgeCraftIcon.tsx (1)
6-10: Hide decorative SVGs from assistive tech.On Line 6, this icon component appears decorative; without explicit a11y attributes, screen readers can still encounter it. Add
aria-hiddenandfocusabledefaults.♿ Suggested tweak
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 48 48" className={className} + aria-hidden="true" + focusable="false" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/common/icons/DropForgeCraftIcon.tsx` around lines 6 - 10, The DropForgeCraftIcon SVG is decorative and needs accessibility attributes: update the DropForgeCraftIcon component so the <svg> element includes aria-hidden="true" and focusable="false" by default (set them directly on the <svg> element or via the component's props with those defaults) to prevent screen readers and keyboard focus from exposing the icon.components/drop-forge/launchClaimHelpers.ts (1)
8-8: Consider rejecting non-positive edition sizes as “missing required”.Line 8 currently allows
0or negative values. If launch requires a positive edition size, this check should guard that too.🛡️ Defensive tweak
- const noEditionSize = claim.edition_size == null; + const noEditionSize = + claim.edition_size == null || claim.edition_size <= 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/launchClaimHelpers.ts` at line 8, The current check "const noEditionSize = claim.edition_size == null" treats 0 or negative values as present; update the guard to treat non-positive sizes as missing by changing it to check for null/undefined or <= 0 (e.g. const noEditionSize = claim.edition_size == null || claim.edition_size <= 0) so that functions using noEditionSize reject zero/negative edition_size values.components/layout/sidebar/WebSidebarNav.tsx (2)
97-102: No-opuseMemo—allSectionsis already memoized.
useSidebarSectionsreturns a memoized array. Wrapping it inuseMemo(() => allSections, [allSections])is an identity function that adds no value.♻️ Simplify
- const allSections = useSidebarSections( + const sections = useSidebarSections( appWalletsSupported, capacitor.isIos, country ); - const sections = useMemo(() => allSections, [allSections]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/layout/sidebar/WebSidebarNav.tsx` around lines 97 - 102, The variable sections is redundantly created by memoizing allSections again; remove the no-op useMemo and use the memoized result from useSidebarSections directly. Replace the lines that declare allSections and sections (the call to useSidebarSections and the useMemo wrapping it) so that you only assign the hook result to a single variable (e.g., allSections or sections) and update any references to use that single identifier; remove the useMemo import/usage tied to this change.
45-45:_isMobileis accepted but unused.The
isMobileprop is destructured as_isMobileand never referenced. If it's planned for future use, consider removing it until needed to keep the API surface clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/layout/sidebar/WebSidebarNav.tsx` at line 45, The component WebSidebarNav currently destructures the prop as _isMobile but never uses it; update the parameter list in the WebSidebarNav functional component (the arrow function taking ({ isCollapsed = false, isMobile: _isMobile = false }, ref)) to either remove the unused isMobile/_isMobile from the destructuring or rename and use the prop where needed; ensure you only change the destructuring so it becomes ({ isCollapsed = false }, ref) if you remove it, or replace _isMobile with isMobile and reference it inside the component (e.g., to adjust rendering) if it’s required.components/drop-forge/DropForgeStatusPill.tsx (1)
29-29: Nit: Consider acn/clsxutility for className concatenation.The inline template literal with conditional classes can become hard to scan. If the project already uses a
cnorclsxhelper, this would be a cleaner fit.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeStatusPill.tsx` at line 29, Replace the long template literal in the DropForgeStatusPill component's JSX className with the project's className helper (e.g., cn or clsx): import the helper at the top, then call cn(...) to combine the base classes, the conditional "tw-cursor-help" when tooltipText is truthy, and the incoming className prop; target the className attribute in the DropForgeStatusPill component to make the JSX cleaner and easier to read.components/the-memes/TheMemesMint.tsx (1)
34-43: Nit:useMemodepends on entirenftobject.If
nftreference isn't stable across renders, the memo provides no benefit. Consider narrowing:♻️ Narrower deps
const mintMetadata = useMemo<ManifoldMintMetadata>( () => ({ tokenId: nft.id, metadata: nft.metadata && typeof nft.metadata === "object" ? (nft.metadata as ArweaveMetadata) : {}, }), - [nft] + [nft.id, nft.metadata] );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/the-memes/TheMemesMint.tsx` around lines 34 - 43, The useMemo for mintMetadata currently lists the entire nft object as a dependency, which defeats memoization if nft's reference changes; update the dependency list for the useMemo that creates mintMetadata to depend only on stable values used inside (e.g., nft.id and the specific metadata piece such as nft.metadata or a serialized form like JSON.stringify(nft.metadata)) so the memo only invalidates when those actual values change; adjust the dependency array in the mintMetadata useMemo accordingly.hooks/useDropForgePermissions.ts (1)
51-58:seizeSettingsreference is unstable and causes unnecessary memo recomputation on every settings fetch.The
SeizeSettingsContextprovider recreates the entireseizeSettingsobject on every fetch (viasetSeizeSettings({...settings, ...})), invalidating the memo dependency. However, the suggested refactor to depend on specific properties (distribution_admin_wallets,claims_admin_wallets) won't solve the issue—those properties are also recreated with new references on every fetch.A more effective approach would be to memoize the seizeSettings object itself in the provider or use a deep comparison in the dependency array, rather than swapping to specific properties that remain equally unstable.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useDropForgePermissions.ts` around lines 51 - 58, The hook useDropForgePermissions currently lists seizeSettings as a dependency but seizeSettings is unstable because the SeizeSettingsContext provider recreates the object on each fetch; to fix this, make seizeSettings stable in the provider (wrap the object returned from SeizeSettingsContext in useMemo or ensure its nested arrays like distribution_admin_wallets and claims_admin_wallets have stable references) so useDropForgePermissions can safely depend on it, or alternatively replace the dependency with a deep-compare effect inside useDropForgePermissions (e.g., useDeepCompareEffect) to avoid recomputation; update either the SeizeSettingsContext provider (memoize the seizeSettings object) or the useDropForgePermissions hook (use deep comparison) and reference seizeSettings, SeizeSettingsContext, and useDropForgePermissions when making the change.components/waves/memes/MemesArtSubmissionTraits.tsx (1)
79-105: Remove redundantreadOnlyre-derivation —getFormSectionsalready applies the override.
getFormSections(userProfile, readOnlyOverrides)mergesreadOnlyOverridesinto each field'sreadOnlybefore returning. Lines 80–87 unnecessarily re-derive the same value and spread a new object on every render, which is wasteful even thoughTraitField's memoization comparison uses value equality onreadOnly.Simplify by passing the field directly (which already has the override applied) and only computing the
readOnlyOverrideboolean prop:♻️ Simplified rendering
{section.fields.map((field, fieldIndex) => { - const hasReadOnlyOverride = - readOnlyOverrides?.[field.field] !== undefined; - const effectiveReadOnly = - readOnlyOverrides?.[field.field] ?? field.readOnly; - const definitionWithReadOnly = - effectiveReadOnly === undefined - ? field - : { ...field, readOnly: effectiveReadOnly }; return ( <TraitField key={`field-${field.field}-${fieldIndex}`} - definition={definitionWithReadOnly} - {...(hasReadOnlyOverride - ? { readOnlyOverride: Boolean(effectiveReadOnly) } + definition={field} + {...(readOnlyOverrides?.[field.field] !== undefined + ? { readOnlyOverride: Boolean(field.readOnly) } : {})} traits={traits}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/MemesArtSubmissionTraits.tsx` around lines 79 - 105, getFormSections already merges readOnlyOverrides into each field, so remove the redundant re-derivation and object spread around field; stop creating hasReadOnlyOverride/effectiveReadOnly/definitionWithReadOnly and instead pass the original field directly into TraitField (definition={field}), and only compute the boolean prop for readOnlyOverride (e.g. when readOnlyOverrides?.[field.field] !== undefined, pass readOnlyOverride={Boolean(field.readOnly)}); update the render loop around TraitField accordingly to avoid recreating objects on every render.components/drop-forge/DropForgeClaimsListPageClient.tsx (1)
44-48: Prefer shared video URL detection helper instead of duplicating logic.This local
isVideoUrlis narrower thanhelpers/video.helpers.tsand can drift from the rest of the app behavior over time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeClaimsListPageClient.tsx` around lines 44 - 48, Replace the local narrow isVideoUrl implementation with the shared helper from helpers/video.helpers.ts: remove the isVideoUrl function in DropForgeClaimsListPageClient.tsx, import the canonical video-detection function (use the exported name from helpers/video.helpers.ts) and call that helper wherever isVideoUrl was used so this component uses the app-wide logic and type signature rather than duplicating detection logic.components/waves/memes/traits/NumberTrait.tsx (1)
167-170: Consider aligning “filled” state with the component’s configured bounds.
n > 0hardcodes positivity even though this input is parameterized bymin/max. Using the same validity criteria as the blur/change paths would keep filled-state behavior consistent across numeric trait variants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/traits/NumberTrait.tsx` around lines 167 - 170, The isFieldFilled computation currently treats any positive number as "filled" (Number.isFinite(n) && n > 0) which ignores the component's min/max configuration; update isFieldFilled to parse currentInputValue and mark filled only when the parsed number is finite and within the configured bounds (respecting min and max) — or reuse the same validation routine used by the blur/change handlers (the existing onBlur/onChange validation function) so filled-state logic matches change/blur validity; reference isFieldFilled, currentInputValue, min, max and the component's blur/change validation when making the change.components/waves/memes/traits/TraitField.tsx (1)
42-42: Simplify redundant placeholder conditional.Line 42 is inside the
FieldType.TEXTbranch, so the ternary is always true there. You can passdefinition.placeholderdirectly for clarity.♻️ Suggested cleanup
- placeholder={definition.type === FieldType.TEXT ? definition.placeholder : undefined} + placeholder={definition.placeholder}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/traits/TraitField.tsx` at line 42, Inside TraitField.tsx, simplify the redundant ternary in the FieldType.TEXT branch by passing definition.placeholder directly instead of using placeholder={definition.type === FieldType.TEXT ? definition.placeholder : undefined}; update the JSX where placeholder is set (within the block handling FieldType.TEXT) to use placeholder={definition.placeholder} so the code is clearer and avoids the always-true conditional.__tests__/components/manifold-minting/ManifoldMintingWidget.test.tsx (1)
59-88: Isolate test cases by clearing mock call history per test.Add a
beforeEach(() => jest.clearAllMocks())inside thedescribeblock to prevent call-count leakage as this suite grows.🧪 Suggested hardening
describe("ManifoldMintingWidget", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + it("shows mint button after address provided", async () => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/manifold-minting/ManifoldMintingWidget.test.tsx` around lines 59 - 88, Add test isolation by inserting a beforeEach hook inside the describe("ManifoldMintingWidget", ...) block that calls jest.clearAllMocks(); this will reset mock call histories (including writeContract) before each it test so counts don't leak between tests; locate the describe block in __tests__/components/manifold-minting/ManifoldMintingWidget.test.tsx and add beforeEach(() => jest.clearAllMocks()) immediately inside it.components/drop-forge/DropForgeAccordionSection.tsx (1)
30-39:showHeaderRightWhenOpenandshowHeaderRightWhenClosedare mutually exclusive due toelse if.If a caller passes both
showHeaderRightWhenOpen={true}andshowHeaderRightWhenClosed={true}(intending "always show"), only theshowHeaderRightWhenOpenbranch executes, so the header-right content hides when closed. Current callers don't set both, so this is fine in practice — just documenting the precedence for future maintainers.If "always show when both flags are true" is desired
let showHeaderRight = false; if (headerRight) { - if (showHeaderRightWhenOpen) { - showHeaderRight = isOpen; - } else if (showHeaderRightWhenClosed) { - showHeaderRight = !isOpen; - } else { + if (showHeaderRightWhenOpen && showHeaderRightWhenClosed) { + showHeaderRight = true; + } else if (showHeaderRightWhenOpen) { + showHeaderRight = isOpen; + } else if (showHeaderRightWhenClosed) { + showHeaderRight = !isOpen; + } else { showHeaderRight = true; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeAccordionSection.tsx` around lines 30 - 39, The current logic in DropForgeAccordionSection that sets showHeaderRight uses an else-if chain so when both props showHeaderRightWhenOpen and showHeaderRightWhenClosed are true it only honors the first; change the condition around headerRight to explicitly handle the both-true case first (treat it as always show), then handle the individual flags using showHeaderRightWhenOpen, showHeaderRightWhenClosed and isOpen to compute showHeaderRight; ensure you still respect headerRight being falsy to keep showHeaderRight=false when headerRight is not provided.__tests__/components/manifold-minting/ManifoldMintingConnect.test.tsx (2)
109-120:ifguards around mock callbacks can make assertions vacuous.Lines 113–115 silently skip calling
mockOnWalletSelectif it'snull(i.e.,RecipientSelectorwasn't rendered). If the component doesn't renderRecipientSelectorin the "mint for me" mode, this test passes without actually exercising the wallet-switch path — thewaitForat Line 117 would then time out or check a stale call.This same pattern appears at Lines 127, 137, 166, and 181. Replace the
ifguards with assertions to make failures explicit:Proposed fix (example for this test)
- if (mockOnWalletSelect) { - mockOnWalletSelect(alternateWallet); - } + expect(mockOnWalletSelect).not.toBeNull(); + mockOnWalletSelect!(alternateWallet);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/manifold-minting/ManifoldMintingConnect.test.tsx` around lines 109 - 120, The test silently skips calling mockOnWalletSelect which makes the assertion vacuous; replace the conditional guard with an explicit assertion that mockOnWalletSelect is defined before invoking it (e.g., assert mockOnWalletSelect is truthy then call mockOnWalletSelect(alternateWallet)) so failures are explicit; apply the same change for other occurrences referenced in this file (the calls around lines referencing RecipientSelector / mockOnWalletSelect at the other tests), ensuring you call mockOnWalletSelect only after asserting its presence and then waitFor expect(onMintFor).toHaveBeenLastCalledWith(alternateWallet).
68-85:connectedProfilemock may be missingwalletsarray.Line 75 provides
connectedProfilewith onlyhandle,display,level, andcic. IfManifoldMintingConnectaccessesconnectedProfile.wallets(e.g., to list user wallets for the "mint for me" selector), the missing property could cause unexpected behavior or mask bugs. Consider adding awalletsarray to the mock.Suggested enhancement
const auth = { - connectedProfile: { handle: "bob", display: "bob", level: 1, cic: 1 }, + connectedProfile: { + handle: "bob", + display: "bob", + level: 1, + cic: 1, + wallets: [{ wallet: "0xabc000000000000000000000000000000000abcd" }], + }, } as any;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/manifold-minting/ManifoldMintingConnect.test.tsx` around lines 68 - 85, The test helper renderConnected provides a connectedProfile without a wallets array which can hide bugs when ManifoldMintingConnect reads connectedProfile.wallets; update the mock in renderConnected to include a realistic wallets array (e.g., one or more wallet objects with expected fields) so tests exercise the wallet-dependent code paths in ManifoldMintingConnect and any wallet-selection logic; adjust the returned auth object used by AuthContext.Provider inside renderConnected to include connectedProfile.wallets and ensure any assertions or mocks that rely on wallet structure are updated accordingly.components/distribution-plan-tool/review-distribution-plan/table/ReviewDistributionPlanTableSubscriptionFooter.tsx (1)
374-380: Array reference in dependency array may cause unnecessary effect re-runs.
seizeSettings.distribution_admin_walletsis a new array reference on every settings fetch, sinceSeizeSettingsContextrecreates theseizeSettingsobject without memoizing individual properties. The effect at line 356 will re-run on every settings update even when the wallets haven't changed. Consider memoizing thedistribution_admin_walletsarray in the context provider (e.g., viauseMemo), or switch to a stable serialized key likeJSON.stringify(seizeSettings.distribution_admin_wallets)for comparison.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/distribution-plan-tool/review-distribution-plan/table/ReviewDistributionPlanTableSubscriptionFooter.tsx` around lines 374 - 380, The effect in ReviewDistributionPlanTableSubscriptionFooter that lists seizeSettings.distribution_admin_wallets in its dependency array is re-running unnecessarily because SeizeSettingsContext recreates seizeSettings (and thus distribution_admin_wallets) on each fetch; to fix, either memoize the distribution_admin_wallets array inside the SeizeSettingsContext provider using useMemo so the reference is stable, or change the dependency in the useEffect inside the ReviewDistributionPlanTableSubscriptionFooter component to a stable serialized key such as JSON.stringify(seizeSettings.distribution_admin_wallets) (refer to seizeSettings, SeizeSettingsContext, and distribution_admin_wallets when locating the code).components/drop-forge/DropForgeCraftClaimPageClient.tsx (2)
2096-2167: Minor:onClosein the lightbox effect deps creates churn.The parent passes
() => setExpandedPhoto(null)inline (line 1959), creating a new function reference each render. This causes the escape-key listener to be torn down and re-attached on every parent render while the lightbox is open. Consider wrapping it inuseCallbackin the parent or moving thesetExpandedPhoto(null)call inside the lightbox.Stabilize the onClose callback
In the parent (
DistributionSection), wrap the handler:+ const closeLightbox = useCallback(() => setExpandedPhoto(null), []); ... <DistributionPhotoLightbox photo={expandedPhoto} photoFileName={...} - onClose={() => setExpandedPhoto(null)} + onClose={closeLightbox} />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeCraftClaimPageClient.tsx` around lines 2096 - 2167, The effect in DistributionPhotoLightbox re-attaches the Escape key listener because onClose is passed as a new inline function from the parent; stabilize it by either wrapping the parent's handler in useCallback (the parent’s setExpandedPhoto(...) caller) or move the closing logic into the lightbox: replace uses of the incoming onClose in DistributionPhotoLightbox (the effect and click handlers) with a stable internal function that calls setExpandedPhoto(null) (or call a memoized prop from the parent) so the dependency array no longer includes a changing function reference and the key listener is not repeatedly torn down and re-attached.
1745-1759: Multiple property-name fallbacks suggest the API type could be tighter.
getAllowlistAddressesandgetAllowlistTotalaccesssummary.addresses ?? summary.addresses_countandsummary.total ?? summary.total_spots. The same pattern appears inrenderPhaseSummaryBoxwithtotal_airdrops. IfClaimPhaseSummaryItemgenuinely supports all these variants, adding an adapter/normalizer would centralize the mapping and avoid scattershot fallbacks.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeCraftClaimPageClient.tsx` around lines 1745 - 1759, Create a small normalizer that converts a ClaimPhaseSummaryItem variants into a canonical shape (e.g., { addresses, total, total_airdrops }) and use it from getAllowlistAddresses, getAllowlistTotal and renderPhaseSummaryBox; specifically, add a function (e.g., normalizeClaimPhaseSummary) that reads addresses/addresses_count and total/total_spots (and total_airdrops variants) and returns consistent numeric fields, then have getAllowlistAddresses call normalizeClaimPhaseSummary(summary).addresses and getAllowlistTotal call normalizeClaimPhaseSummary(summary).total (and update renderPhaseSummaryBox to read normalized.total_airdrops) so you centralize the mapping instead of using ?? fallbacks throughout.components/drop-forge/DropForgeLaunchClaimPageClient.tsx (4)
602-705:LaunchAccordionSectionduplicatesDropForgeAccordionSection.The Craft page imports
DropForgeAccordionSectionfrom a shared module, while this file defines a localLaunchAccordionSectionwith overlapping functionality (accordion with tone, headerRight, disabled state). Consider extendingDropForgeAccordionSectionto support the additional props (tone, disabled, onOpen) instead of maintaining two similar implementations.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeLaunchClaimPageClient.tsx` around lines 602 - 705, Remove the local LaunchAccordionSection implementation and reuse the shared DropForgeAccordionSection by adding the missing props and behavior to it: extend DropForgeAccordionSection to accept tone, disabled, onOpen, headerRight, showHeaderRightWhenOpen and defaultOpen, implement the same open/close state logic (including calling onOpen when opening), aria/disabled handling, icon rotation classes (reuse toneClass for the subtitle badge), and the same collapsed/expanded DOM structure (SECTION_CARD_CLASS and children wrapper) so all callers of LaunchAccordionSection can import DropForgeAccordionSection instead; update any local uses to pass the new props and remove the duplicated LaunchAccordionSection symbol.
174-194: Consider typing the API response instead ofas unknown ascasts.
getRootAddressesCountandgetRootTotalSpotsboth use double-cast (as unknown as) to accessaddresses_count/addressesandtotal_spots/total. This works but hides the actual shape ofMintingClaimsRootItem. If the generated type can be extended, adding those optional fields toMintingClaimsRootItemwould be safer and more discoverable.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeLaunchClaimPageClient.tsx` around lines 174 - 194, The helper functions getRootAddressesCount and getRootTotalSpots are using double-casts to access properties that should be part of the MintingClaimsRootItem shape; instead extend or update the MintingClaimsRootItem type to include optional fields addresses_count, addresses, total_spots, and total, then remove the "as unknown as" casts and read those properties directly in getRootAddressesCount and getRootTotalSpots (keep the same null checks and typeof number guards). Update the API/type generation or the local type declaration where MintingClaimsRootItem is defined so the compiler sees those optional fields.
1016-1042: Effect with state objects in the dependency array could cause redundant runs.
subscriptionAirdropsByPhaseandsubscriptionAirdropsLoadingByPhaseare objects in the dependency array, and every state update creates a new object reference. The guard at lines 1029–1031 prevents actual re-fetching, so this is safe but causes the effect body to execute on each state change. Not a correctness issue, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeLaunchClaimPageClient.tsx` around lines 1016 - 1042, The effect runs on every state update because object references subscriptionAirdropsByPhase and subscriptionAirdropsLoadingByPhase are in the dependency array; change the dependencies to only the specific primitive values the effect cares about (e.g. selectedPhase plus the lookup results for that phase) or derive those lookups with useMemo and depend on the memoized primitives instead; update the useEffect that references selectedPhase, subscriptionAirdropsByPhase, subscriptionAirdropsLoadingByPhase, and fetchSubscriptionAirdropsForPhase so it depends on selectedPhase and subscriptionAirdropsByPhase[selectedPhase] / subscriptionAirdropsLoadingByPhase[selectedPhase] (or their memoized equivalents) to avoid redundant runs while keeping the existing guard and fetchSubscriptionAirdropsForPhase call intact.
707-770: Consider splitting this ~2600-line file into smaller modules.The main component alone has ~35
useStatehooks and ~15useEffecthooks. Extracting the utility functions (lines 101–600) to a shared helpers module, and theClaimTransactionModalandLaunchAccordionSectionto separate component files, would improve maintainability and testability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeLaunchClaimPageClient.tsx` around lines 707 - 770, The DropForgeLaunchClaimPageClient component is too large and hard to maintain; split it into smaller modules by extracting utility functions and heavy subcomponents: move helper functions (the utilities used by DropForgeLaunchClaimPageClient, originally in the top-of-file region) into a new helpers module and import them into DropForgeLaunchClaimPageClient, and extract the ClaimTransactionModal and LaunchAccordionSection JSX/logic into standalone components (e.g., ClaimTransactionModal.tsx and LaunchAccordionSection.tsx) that accept props/state handlers from DropForgeLaunchClaimPageClient; ensure you keep the same prop and ref names (e.g., claim, setClaim, claimTxModal, setClaimTxModal, handledClaimWriteSuccessTxHashRef) and move related state-management hooks only when appropriate (lift shared state into the parent and pass handlers down) so DropForgeLaunchClaimPageClient remains the orchestrator while the new modules contain presentation and helper logic.
There was a problem hiding this comment.
Actionable comments posted: 12
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/manifold-minting/ManifoldMintingWidget.tsx (1)
413-424:⚠️ Potential issue | 🟡 Minor
Number.parseInt("")returnsNaN, which can crashBigInt()at mint time.If the user clears this input,
mintCountbecomesNaN. Later,getValue()returnsNaNandBigInt(NaN)throws aTypeError. Consider defaulting to 0 on parse failure.Proposed fix
- onChange={(e) => setMintCount(Number.parseInt(e.target.value))} + onChange={(e) => { + const parsed = Number.parseInt(e.target.value); + setMintCount(Number.isNaN(parsed) ? 0 : parsed); + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifold-minting/ManifoldMintingWidget.tsx` around lines 413 - 424, The input handler in printMintCountInput can set mintCount to NaN when the field is cleared, which later makes getValue() return NaN and BigInt(NaN) throw; change the onChange in printMintCountInput to parse the value and default to 0 on failure (e.g., const v = Number.parseInt(e.target.value); setMintCount(Number.isNaN(v) ? 0 : v)), and also add a defensive fallback in the code that uses mintCount (getValue()/where BigInt is called) to use 0 when mintCount is falsy or NaN (e.g., BigInt(Number.isNaN(mintCount) ? 0 : mintCount)) so BigInt never receives NaN.
♻️ Duplicate comments (7)
__tests__/hooks/useManifoldClaim.test.tsx (1)
38-51: Past issue resolved — hook now correctly invoked with object params and claims accessed viaresult.current.claim.The test correctly uses the new API shape and the assertions target the nested
claimproperty. The mock data structure (tuple with[instanceId, claimObject]) aligns with thegetClaimForTokenpath (chainId=1 → mainnet).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/hooks/useManifoldClaim.test.tsx` around lines 38 - 51, Update the test to invoke useManifoldClaim with the object-shaped params and assert the nested claim fields: ensure the renderHook call uses useManifoldClaim({ chainId: 1, contract: "0x1", proxy: "0x2", abi: [], identifier: 1 }) and that assertions read from result.current.claim (e.g., expect(result.current.claim?.status).toBe(ManifoldClaimStatus.ACTIVE), expect(result.current.claim?.phase).toBe(ManifoldPhase.PUBLIC), expect(result.current.claim?.remaining).toBe(1)); also ensure your mock for getClaimForToken returns the tuple [instanceId, claimObject] for chainId 1 so the hook resolves the correct claim shape.components/providers/AppKitAdapterManager.ts (2)
14-14:⚠️ Potential issue | 🟠 Major
Chaintype must be imported fromviem, notviem/chains.Same incorrect import as was fixed in
WagmiSetup.tsxand flagged inutils/appkit-initialization.utils.ts.viem/chainsexports chain instances; theChainTypeScript type belongs at theviemroot.mainnetstays inviem/chains.🐛 Proposed fix
-import { Chain, mainnet } from "viem/chains"; +import type { Chain } from "viem"; +import { mainnet } from "viem/chains";Does viem/chains export the Chain type, or is it only exported from the viem root package?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/providers/AppKitAdapterManager.ts` at line 14, The import currently pulls the Chain type from "viem/chains"; update the import so the TypeScript type Chain is imported from the viem root and mainnet remains imported from "viem/chains" — locate the import line in AppKitAdapterManager.ts (referencing Chain and mainnet) and split it into two imports: one importing Chain from "viem" and another importing mainnet from "viem/chains".
14-14:⚠️ Potential issue | 🟠 Major
Chaintype must be imported fromviem, notviem/chains.Same incorrect import as
utils/appkit-initialization.utils.tsLine 11 and the prior issue already fixed inWagmiSetup.tsx. The canonical form shown in viem/Wagmi docs isimport { type Chain } from 'viem'—viem/chainsexports chain objects, not theChaintype.mainnetcorrectly stays inviem/chains.🐛 Proposed fix
-import { Chain, mainnet } from "viem/chains"; +import type { Chain } from "viem"; +import { mainnet } from "viem/chains";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/providers/AppKitAdapterManager.ts` at line 14, The import is pulling the Chain type from the wrong module; change the import so the type symbol Chain is imported from 'viem' while keeping mainnet imported from 'viem/chains' (i.e., replace import of Chain from "viem/chains" with importing type Chain from "viem" and leave mainnet from "viem/chains") so references to the Chain type in AppKitAdapterManager (and any other places using Chain) use the correct type definition.components/header/AppUserConnect.tsx (1)
47-56: Chain-switching button integration looks good.The conditional rendering, aria-label, and use of the centralized
useChainSwitcherhook are clean. This properly addresses the prior review feedback about cycling through all chains.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/header/AppUserConnect.tsx` around lines 47 - 56, No change required: the chain-switching button correctly conditionally renders when chains.length > 1, uses the centralized useChainSwitcher via switchToNextChain, includes an accessible aria-label "Switch Chain", displays currentChainName, and uses ArrowsRightLeftIcon; keep this implementation as-is.components/layout/sidebar/WebSidebarNav.tsx (1)
48-48: Drop Forge sidebar integration usinguseDropForgePermissions— LGTM.This properly uses the consolidated
canAccessLandingfromuseDropForgePermissions, addressing the prior review feedback about duplicating permission checks. The nav item is correctly gated and positioned.Also applies to: 349-362
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/layout/sidebar/WebSidebarNav.tsx` at line 48, No code changes required: the Drop Forge sidebar integration in WebSidebarNav.tsx correctly uses the consolidated permission hook useDropForgePermissions and its canAccessLanding alias (showDropForge) to gate the nav item; keep the existing const { canAccessLanding: showDropForge } = useDropForgePermissions() and the conditional rendering around the Drop Forge nav entry as-is.openapi.yaml (2)
2405-2409:⚠️ Potential issue | 🟡 MinorRequire a non-empty PATCH payload for
patchMintingClaim.At Line 2405 the
requestBodyis not required, and at Line 11713MintingClaimUpdateRequesthas no required fields orminProperties, so empty/no-op PATCH requests are valid.Proposed fix
/minting-claims/{contract}/claims/{claim_id}: patch: requestBody: + required: true content: application/json: schema: $ref: "#/components/schemas/MintingClaimUpdateRequest" MintingClaimUpdateRequest: type: object + minProperties: 1 properties: edition_size: type: integer#!/bin/bash # Verify requestBody required flag and minProperties for PATCH schema sed -n '2398,2413p' openapi.yaml sed -n '11710,11740p' openapi.yamlAlso applies to: 11712-11737
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 2405 - 2409, The PATCH operation patchMintingClaim currently allows empty payloads because its requestBody is not marked required and the schema MintingClaimUpdateRequest has no required fields or minProperties; update the operation to set requestBody.required: true for patchMintingClaim and modify the MintingClaimUpdateRequest schema to include either specific required properties or add minProperties: 1 so a non-empty JSON object is enforced.
11510-11557:⚠️ Potential issue | 🟠 Major
animation_detailsunion is still ambiguous for validators/codegen.At Line 11511,
oneOfhas no discriminator, and at Line 11556 the videoformatis unconstrained (type: string). This can produce nondeterministic matching.Proposed fix
animation_details: oneOf: - $ref: "#/components/schemas/MintingClaimAnimationDetailsVideo" - $ref: "#/components/schemas/MintingClaimAnimationDetailsHtml" - $ref: "#/components/schemas/MintingClaimAnimationDetailsGlb" + discriminator: + propertyName: format + mapping: + VIDEO: "#/components/schemas/MintingClaimAnimationDetailsVideo" + HTML: "#/components/schemas/MintingClaimAnimationDetailsHtml" + GLB: "#/components/schemas/MintingClaimAnimationDetailsGlb" nullable: true MintingClaimAnimationDetailsVideo: type: object @@ format: type: string + enum: + - VIDEO#!/bin/bash # Verify oneOf/discriminator and video format constraints sed -n '11506,11520p' openapi.yaml sed -n '11540,11562p' openapi.yaml🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 11510 - 11557, The oneOf union animation_details is ambiguous for validators/codegen; add an OpenAPI discriminator on the parent schema and constrain the video format so type resolvers can pick the correct variant: update the animation_details schema to include a discriminator with propertyName: "format" and mapping entries pointing format values to the concrete schemas (e.g., "GLB" -> "#/components/schemas/MintingClaimAnimationDetailsGlb", "HTML" -> "#/components/schemas/MintingClaimAnimationDetailsHtml", and the chosen video format tokens -> "#/components/schemas/MintingClaimAnimationDetailsVideo"), then modify MintingClaimAnimationDetailsVideo.format to be an enum of the allowed video formats (e.g., MP4, WEBM, MOV or whatever your system accepts) so the discriminator values match the mapping and the oneOf selection is deterministic.
🟡 Minor comments (10)
components/manifold-minting/ManifoldMintingWidget.tsx-318-328 (1)
318-328:⚠️ Potential issue | 🟡 MinorMissing short-error fallback — inconsistent with
mintWrite.errorhandler.The
mintWrite.errorhandler (lines 302–316) checksresolvedError.length < 5and falls back to the full message for overly short/empty parsed strings. This handler lacks the same guard, so ifwaitMintWrite.error.messageparses down to an empty or very short string, the user will see a blank or unhelpful error.Proposed fix
useEffect(() => { if (waitMintWrite.error) { setMintStatus(<></>); const resolvedError = waitMintWrite.error.message ?.split("Request Arguments")[0] ?.split(".")[0] - ?.split("Contract Call")[0] ?? waitMintWrite.error.message; - setMintError(resolvedError); + ?.split("Contract Call")[0]; + if (!resolvedError || resolvedError.length < 5) { + setMintError(waitMintWrite.error.message); + } else { + setMintError(resolvedError); + } } }, [waitMintWrite.error]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifold-minting/ManifoldMintingWidget.tsx` around lines 318 - 328, The waitMintWrite.error handler builds a parsed resolvedError from waitMintWrite.error.message but lacks the same short-string fallback used by the mintWrite.error handler; update the useEffect handling waitMintWrite.error (the block that calls setMintStatus and setMintError) to check if the parsed resolvedError length is less than 5 (or empty) and in that case fall back to the original waitMintWrite.error.message before calling setMintError so users don’t see a blank/too-short message.components/navigation/NavItem.tsx-47-54 (1)
47-54:⚠️ Potential issue | 🟡 MinorReset the title when unread notifications become zero.
At Line 49-54, the effect sets title only in the unread branch. If unread transitions from
>0to0, the previous"(N) Notifications | 6529.io"title can persist until another setter overrides it.Suggested adjustment
useEffect(() => { if (item.name !== "Notifications") return; - if (haveUnreadNotifications) { - setTitle(`(${notifications?.unread_count}) Notifications | 6529.io`); - } - if (!haveUnreadNotifications) { + const unreadCount = notifications?.unread_count ?? 0; + if (haveUnreadNotifications && unreadCount > 0) { + setTitle(`(${unreadCount}) Notifications | 6529.io`); + } else { + setTitle("6529.io"); removeAllDeliveredNotifications(); } }, [🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/navigation/NavItem.tsx` around lines 47 - 54, The useEffect watching notifications only sets the document title when haveUnreadNotifications is true, so when unread transitions to 0 the previous "(N) Notifications | 6529.io" can remain; update the effect in NavItem (the useEffect that checks item.name === "Notifications") to explicitly reset the title (e.g., to "Notifications | 6529.io") when haveUnreadNotifications is false (in the same branch that calls removeAllDeliveredNotifications), ensuring you still call removeAllDeliveredNotifications() and use notifications?.unread_count when constructing the title in the unread branch.components/drops/view/item/content/media/MediaDisplayGLB.tsx-130-131 (1)
130-131:⚠️ Potential issue | 🟡 MinorAvoid static tooltip IDs in a reusable component.
"glb-cube-tooltip"is hard-coded for both the trigger and tooltip (Line [130], Line [173]). If multipleMediaDisplayGLBinstances render simultaneously, IDs collide and tooltip targeting becomes unreliable. Please generate a per-instance id and reuse it in both places.Also applies to: 173-174
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/media/MediaDisplayGLB.tsx` around lines 130 - 131, The component MediaDisplayGLB uses a hard-coded tooltip ID ("glb-cube-tooltip") for both the trigger and the tooltip which will collide when multiple instances mount; change it to generate a per-instance ID (e.g., with React's useId() or a useRef(nanoid()) inside MediaDisplayGLB) and replace all occurrences of the static string (data-tooltip-id and the tooltip id prop) with that generated id so each instance targets its own tooltip.components/drops/view/item/content/media/MediaDisplayGLB.tsx-122-131 (1)
122-131:⚠️ Potential issue | 🟡 MinorSet
type="button"on the cube toggle.Line [122] omits an explicit button type. Inside a form, this defaults to submit and can trigger unintended form submission.
💡 Proposed fix
<button onClick={handleCubeToggle} + type="button" className={`tw-flex tw-h-9 tw-w-9 tw-items-center tw-justify-center tw-rounded-full tw-border tw-border-solid tw-transition-all tw-duration-200 ${ isActive ? "tw-border-primary-600 tw-bg-primary-500 tw-text-white tw-shadow-lg tw-shadow-primary-500/50 hover:tw-bg-primary-600"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/media/MediaDisplayGLB.tsx` around lines 122 - 131, The toggle button in the MediaDisplayGLB component lacks an explicit type which makes it act as a submit button inside forms; update the button element used for the cube toggle (the one with onClick={handleCubeToggle} and data-tooltip-id="glb-cube-tooltip") to include type="button" so it won't trigger form submission when clicked.components/drop-forge/DropForgeLaunchClaimPageClient.tsx-286-286 (1)
286-286:⚠️ Potential issue | 🟡 MinorUse a trimmed check for published metadata state.
Line 286 treats empty-string metadata as published. This can expose launch controls prematurely.
🔧 Suggested fix
- const hasPublishedMetadata = Boolean(claim?.metadata_location != null); + const hasPublishedMetadata = Boolean(claim?.metadata_location?.trim());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeLaunchClaimPageClient.tsx` at line 286, The current published-metadata check in hasPublishedMetadata uses Boolean(claim?.metadata_location != null) which treats an empty string as published; update the check to ensure claim?.metadata_location is a non-empty, non-whitespace string by trimming and testing length (e.g., use claim?.metadata_location?.trim() and verify it's not an empty string) so hasPublishedMetadata only becomes true when metadata_location contains real content.components/drop-forge/DropForgeCraftClaimPageClient.tsx-1483-1485 (1)
1483-1485:⚠️ Potential issue | 🟡 MinorTreat empty metadata values as unpublished.
Line 1484 considers
""as published metadata. Use a trimmed non-empty check for consistency with launch-status logic.🔧 Suggested fix
- const hasPublishedMetadata = claim.metadata_location != null; + const hasPublishedMetadata = Boolean(claim.metadata_location?.trim());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeCraftClaimPageClient.tsx` around lines 1483 - 1485, The check for published metadata treats an empty string as published; update hasPublishedMetadata so it verifies claim.metadata_location is non-null and non-empty after trimming (e.g., use a trimmed non-empty check on claim.metadata_location) to match the launch-status logic; ensure any downstream logic that uses hasPublishedMetadata (such as canPublish) uses this updated boolean.components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts-306-329 (1)
306-329:⚠️ Potential issue | 🟡 Minor
getAnimationMimeTypeunconditionally falls back to"video/mp4"for unrecognised formats.If
formatis a non-video string (e.g."UNKNOWN") that isn't informatMimeMap, line 322 returns"video/mp4". Similarly, the final return on line 328 assumes video for any URL that doesn't match known extensions. This could cause incorrect<source type="…">attributes for non-video content, leading to playback failures or misleading metadata.Consider returning
nullfor truly unrecognised formats so callers can decide how to handle unknown media.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts` around lines 306 - 329, The function getAnimationMimeType currently defaults to "video/mp4" for unknown formats and URLs; change it to return null for truly unrecognised media so callers can handle unknown types. Specifically, in getAnimationMimeType when reading claim.animation_details.format (and after checking "HTML" and "GLB"), if format exists but is not in formatMimeMap (and not matched by normalizedFormat), return null instead of "video/mp4"; likewise, after the extension checks and isVideoUrl check, return null as the final fallback rather than "video/mp4". Keep existing checks for HTML/GLB, formatMimeMap, isVideoUrl, and the .endsWith(...) extension checks but change the default returns to null.components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts-344-352 (1)
344-352:⚠️ Potential issue | 🟡 Minor
toArweaveUrldoesn't validate the Arweave transaction ID format.Any non-empty, non-URL string will be appended to
https://arweave.net/. Malicious or malformed input (e.g."../../foo","<script>") could produce unexpected URLs. Consider basic validation (e.g. checking the string is a valid base64url Arweave TX ID of 43 characters) or at minimum encoding the path segment.Minimal hardening
if (trimmed.startsWith("http://") || trimmed.startsWith("https://")) { return trimmed; } - return `https://arweave.net/${trimmed}`; + return `https://arweave.net/${encodeURIComponent(trimmed)}`; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts` around lines 344 - 352, The toArweaveUrl function currently appends any non-URL string to https://arweave.net/, so validate the path before returning: inside toArweaveUrl, after computing trimmed, first allow full URLs as now, otherwise verify trimmed matches the Arweave TX ID pattern (a 43-character base64url string) using a regexp (e.g., /^[A-Za-z0-9-_]{43}$/) and only then return `https://arweave.net/${trimmed}`; if it fails validation return null (or alternatively return `https://arweave.net/${encodeURIComponent(trimmed)}` if you prefer encoding over rejecting). Ensure you reference the trimmed variable and keep the early returns for null intact.components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts-48-55 (1)
48-55:⚠️ Potential issue | 🟡 Minor
isNotFoundErrorhas a case-sensitivity inconsistency.Lines 51–52 compare against the lowercased
normalized, but line 53 compares against the originalmessagewith exact casing ("Claim not found"). If the actual message is"claim not found"it won't match line 53, and if it's"Claim not found"it already matches line 51. The exact-case check on line 53 is therefore either dead code or a latent bug.Proposed fix
export function isNotFoundError(message: string): boolean { const normalized = message.toLowerCase(); return ( normalized.includes("not found") || - normalized.includes("status code 404") || - message === "Claim not found" + normalized.includes("status code 404") ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts` around lines 48 - 55, The isNotFoundError function mixes casing by comparing normalized (lowercased) values for two checks but uses the original message for the exact match "Claim not found"; fix by using the normalized string for that comparison (e.g., compare normalized === "claim not found" or remove the redundant exact match since normalized.includes("not found") already covers it) so all comparisons are case-insensitive; update the function isNotFoundError accordingly.components/header/user/proxy/HeaderUserProxyDropdown.tsx-65-66 (1)
65-66:⚠️ Potential issue | 🟡 Minor
useEffectdependency array is incomplete and the pattern can be simplified withuseMemo.
getLabel()also readsactiveProfileProxy(indirectly via its closure), but the effect only depends on[profile, address]. IfactiveProfileProxychanges, the label won't update. Additionally, theuseState+useEffectcombo for derived state is a React anti-pattern —useMemois more appropriate here.Proposed fix
- const [label, setLabel] = useState(getLabel()); - useEffect(() => setLabel(getLabel()), [profile, address]); + const label = useMemo(getLabel, [profile, address, activeProfileProxy]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/header/user/proxy/HeaderUserProxyDropdown.tsx` around lines 65 - 66, The label derivation in HeaderUserProxyDropdown uses useState + useEffect and calls getLabel() which also reads activeProfileProxy via closure, but the effect only depends on [profile, address], so label can become stale; replace the useState/setLabel + useEffect pattern with a useMemo that returns getLabel() (or the same label logic) and include all dependencies (profile, address, activeProfileProxy) in the dependency array so the label updates whenever any of those change.
🧹 Nitpick comments (24)
components/home/now-minting/NowMintingStatsGrid.tsx (1)
33-33: Prefer removing the mainnet fallback here.Line 33 can pass
contractdirectly. The?? MEMES_CONTRACTfallback is redundant with current typing and may mask future config issues.♻️ Suggested cleanup
-import { MEMES_CONTRACT } from "@/constants/constants"; @@ - contract: contract ?? MEMES_CONTRACT, + contract,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/home/now-minting/NowMintingStatsGrid.tsx` at line 33, Remove the mainnet fallback and pass the incoming prop directly: replace the object entry that currently uses "contract: contract ?? MEMES_CONTRACT" with a direct "contract: contract" (or simply rely on the prop where used) inside the NowMintingStatsGrid component so the MEMES_CONTRACT fallback is not masked; update any call sites or prop typings if needed to ensure contract remains required/validated.components/user/subscriptions/UserPageSubscriptionsTopUp.tsx (1)
48-61:confirm_walletandsubmittedshare the same emoji — verify if intentional; also consider extracting the union type.Two separate things worth a quick look:
Shared emoji: Both
confirm_walletandsubmittedresolve to/emojis/sgt_flushed.webp. If different visual feedback was intended for these two states (e.g., a spinner-adjacent face for "pending wallet action" vs. a "transaction in flight" face), this should be updated.Duplicated union type: The literal union
"confirm_wallet" | "submitted" | "success" | "error"is written twice — once in the parameter and again as theRecordkey. Extract it to a named type to keep it DRY.The function wrapper is unnecessary — the body is a static lookup; a
constmap is clearer.♻️ Proposed simplification
-function getTopUpModalEmoji( - status: "confirm_wallet" | "submitted" | "success" | "error" -): string { - const emojiByStatus: Record< - "confirm_wallet" | "submitted" | "success" | "error", - string - > = { - confirm_wallet: "/emojis/sgt_flushed.webp", - submitted: "/emojis/sgt_flushed.webp", - success: "/emojis/sgt_saluting_face.webp", - error: "/emojis/sgt_sob.webp", - }; - return emojiByStatus[status]; -} +type TopUpModalStatus = "confirm_wallet" | "submitted" | "success" | "error"; + +const TOP_UP_MODAL_EMOJI: Record<TopUpModalStatus, string> = { + confirm_wallet: "/emojis/sgt_flushed.webp", + submitted: "/emojis/sgt_flushed.webp", // ← same as confirm_wallet; intentional? + success: "/emojis/sgt_saluting_face.webp", + error: "/emojis/sgt_sob.webp", +};Call sites become
TOP_UP_MODAL_EMOJI["error"], etc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/user/subscriptions/UserPageSubscriptionsTopUp.tsx` around lines 48 - 61, Replace the duplicated union and function with a single named type and constant map: extract the union "confirm_wallet" | "submitted" | "success" | "error" into a type alias (e.g., TopUpModalStatus), create a const TOP_UP_MODAL_EMOJI: Record<TopUpModalStatus,string> containing the emoji paths, and remove getTopUpModalEmoji in favor of direct lookups (TOP_UP_MODAL_EMOJI[status]); while doing this, verify whether confirm_wallet and submitted should share "/emojis/sgt_flushed.webp" and, if not, update the appropriate entry in TOP_UP_MODAL_EMOJI to the intended emoji.components/distribution-plan-tool/review-distribution-plan/table/ReviewDistributionPlanTableSubscriptionFooter.tsx (1)
545-550: Inconsistency: render guard bypasses the null-safedistributionAdminWalletsmemo.The
useEffectguard (line 366) correctly uses the memoizeddistributionAdminWallets(which falls back to[]), but this render guard callsisSubscriptionsAdminwith the rawseizeSettings.distribution_admin_wallets. The two call sites should use the same value for consistency; using the memoized variable here also makes any future null-safety fix automatically apply to both paths.♻️ Proposed fix
- if ( - !isSubscriptionsAdmin( - connectedProfile, - seizeSettings.distribution_admin_wallets - ) - ) { + if (!isSubscriptionsAdmin(connectedProfile, distributionAdminWallets)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/distribution-plan-tool/review-distribution-plan/table/ReviewDistributionPlanTableSubscriptionFooter.tsx` around lines 545 - 550, The render guard currently calls isSubscriptionsAdmin(connectedProfile, seizeSettings.distribution_admin_wallets) bypassing the null-safe memo; update that call to use the memoized distributionAdminWallets instead (i.e., isSubscriptionsAdmin(connectedProfile, distributionAdminWallets)) so both the useEffect and render guard use the same null-safe value and future null-safety fixes apply consistently; locate the check around the if(...) block and replace the second argument accordingly, keeping connectedProfile and the isSubscriptionsAdmin invocation intact.components/manifold-minting/ManifoldMintingWidget.tsx (3)
17-17: Useimport typefor type-only import.
Chainis only used as a type annotation (line 32). Other type-only imports in this file already useimport type(lines 10, 13). This should be consistent.Proposed fix
-import { Chain } from "viem"; +import type { Chain } from "viem";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifold-minting/ManifoldMintingWidget.tsx` at line 17, Change the value import of Chain to a type-only import: replace the current import of Chain with an "import type { Chain }" so it is treated as a type-only import used in the ManifoldMintingWidget component (Chain is only used as a type annotation around the props/variable on line where Chain is referenced). This keeps import style consistent with the other type-only imports in the file and avoids emitting a runtime import for the Chain symbol.
57-96:mintWriteused inside effect but missing from dependency array.Line 58 calls
mintWrite.reset(), butmintWriteis not listed in the dependency array (lines 91–96). This will trigger anreact-hooks/exhaustive-depslint warning. IncludingmintWritedirectly would cause re-runs on every render (wagmi returns a new object), so the common workaround is to extractresetinto a stable ref or suppress the warning with an inline comment. This is a known trade-off and not a bug, but worth acknowledging with a suppression comment to avoid future confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifold-minting/ManifoldMintingWidget.tsx` around lines 57 - 96, The useEffect in ManifoldMintingWidget calls mintWrite.reset() but mintWrite is omitted from the dependency array, triggering react-hooks/exhaustive-deps; fix by either extracting the reset function into a stable reference (e.g., const resetMintRef = useRef(() => mintWrite.reset()) and call resetMintRef.current() inside the effect) or explicitly suppress the lint rule with an inline comment above the dependency array, and keep references to mintWrite.reset, the useEffect block, and the dependency array unchanged otherwise so the intent is clear.
199-202: Redundant?? []after.map()calls.
Array.prototype.map()always returns an array, so the nullish coalescing fallback on lines 200 and 201 is dead code. Removing it improves clarity.Proposed fix
mintArgs.push( - selectedMerkleProofs.map((mp) => mp.value) ?? [], - selectedMerkleProofs.map((mp) => mp.merkle_proof) ?? [] + selectedMerkleProofs.map((mp) => mp.value), + selectedMerkleProofs.map((mp) => mp.merkle_proof) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/manifold-minting/ManifoldMintingWidget.tsx` around lines 199 - 202, The call to Array.prototype.map on selectedMerkleProofs returns an array, so remove the redundant nullish coalescing fallbacks after those maps in the mintArgs.push call; update the mintArgs.push invocation that uses selectedMerkleProofs.map((mp) => mp.value) and selectedMerkleProofs.map((mp) => mp.merkle_proof) to omit the trailing "?? []" so the push uses the mapped arrays directly.__tests__/components/navigation/BottomNavigation.test.tsx (1)
45-69: Add assertions for the newisCurrentWaveDmprop wiring.The test scaffolding mocks
useWave, but it never verifies the value passed toNavItem. Add an assertion for bothfalseandtrueto lock in this PR behavior.Test coverage enhancement
describe('BottomNavigation', () => { it('registers mobileNav ref and renders nav items', () => { @@ const notificationsItem = passedItems.find((item: { name: string }) => item.name === 'Notifications'); expect(notificationsItem?.href).toBe(getNotificationsRoute(false)); + expect(navItemCalls.every((call) => call[0].isCurrentWaveDm === false)).toBe(true); }); + + it('passes DM state to NavItem when current wave is DM', () => { + (useWave as jest.Mock).mockReturnValue({ isDm: true }); + render(<BottomNavigation />); + const navItemCalls = (NavItem as jest.Mock).mock.calls; + expect(navItemCalls).toHaveLength(7); + expect(navItemCalls.every((call) => call[0].isCurrentWaveDm === true)).toBe(true); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/navigation/BottomNavigation.test.tsx` around lines 45 - 69, The test must assert that BottomNavigation wires the useWave value into NavItem via the isCurrentWaveDm prop; update the test in BottomNavigation.test.tsx to inspect (NavItem as jest.Mock).mock.calls for the nav item whose name is 'Messages' (or for all calls) and add expectations that .isCurrentWaveDm is false for the current mocked useWave state, then re-mock useWave to return the alternate state and re-render BottomNavigation and assert .isCurrentWaveDm is true; reference NavItem, useWave, BottomNavigation, and registerRef to locate the code and ensure you check the nav call arguments' isCurrentWaveDm property in both false and true scenarios.components/providers/WagmiSetup.tsx (2)
103-103: Consider hoistingenableTestnetoutside the component.
publicEnvis a module-level constant, soenableTestnetnever changes at runtime. Declaring it inside the component body makes it look like reactive state and adds a redundant entry to theuseCallbackdep array at Line 157.♻️ Proposed refactor
+const enableTestnet = publicEnv.DROP_FORGE_TESTNET === true; + export default function WagmiSetup({ children, }: { readonly children: React.ReactNode; }) { - const enableTestnet = publicEnv.DROP_FORGE_TESTNET === true; - const appWalletPasswordModal = useAppWalletPasswordModal();Then remove
enableTestnetfrom theuseCallbackdependency array:- [adapterManager, isCapacitor, enableTestnet] + [adapterManager, isCapacitor]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/providers/WagmiSetup.tsx` at line 103, Hoist the computed boolean enableTestnet out of the WagmiSetup component to module scope since publicEnv is module-level and never changes; move the declaration next to publicEnv and then remove enableTestnet from the dependency array of the useCallback inside WagmiSetup so the callback deps no longer include a non-reactive value.
103-103: Consider hoistingenableTestnetoutside the component.
publicEnvis a module-level constant, soenableTestnetis a build-time boolean that never changes at runtime. Placing it inside the component body makes it look like reactive state and adds a redundant entry to theuseCallbackdependency array at Line 157.♻️ Proposed refactor
+const enableTestnet = publicEnv.DROP_FORGE_TESTNET === true; + export default function WagmiSetup({ children, }: { readonly children: React.ReactNode; }) { - const enableTestnet = publicEnv.DROP_FORGE_TESTNET === true;Then remove
enableTestnetfrom theuseCallbackdependency array:- [adapterManager, isCapacitor, enableTestnet] + [adapterManager, isCapacitor]🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/providers/WagmiSetup.tsx` at line 103, enableTestnet is computed from module-level publicEnv but is declared inside the WagmiSetup component, making it appear reactive and unnecessarily included in the useCallback dependency array; hoist the constant declaration (const enableTestnet = publicEnv.DROP_FORGE_TESTNET === true) to module scope above the WagmiSetup component, update any references inside WagmiSetup to use the hoisted symbol, and then remove enableTestnet from the dependency array of the affected useCallback (the callback defined in WagmiSetup that currently lists enableTestnet).components/providers/AppKitAdapterManager.ts (2)
264-277:getSortedChainIdentifiersreuses the empty-array error code/message for a structurally different failure.Line 272 throws
"ADAPTER_021: chains must be a non-empty array"when a chain element in a non-empty array has a missing or non-numericid. This is a distinct failure mode from an empty array (Line 265-267) but is surfaced with the same code, making the two indistinguishable in logs.♻️ Proposed fix
return chains .map((chain) => { if (!chain || typeof chain.id !== "number") { - throw new AdapterError("ADAPTER_021: chains must be a non-empty array"); + throw new AdapterError("ADAPTER_022: chains contains an invalid chain object (missing or non-numeric id)"); } return `${chain.id}`; })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/providers/AppKitAdapterManager.ts` around lines 264 - 277, getSortedChainIdentifiers currently throws the same AdapterError ("ADAPTER_021: chains must be a non-empty array") for two different failure modes (empty/non-array input and invalid chain elements), which makes diagnostics ambiguous; change the inner validation that checks each chain element (inside the .map) to throw a distinct AdapterError with a new code/message (e.g., "ADAPTER_022: chain elements must be objects with numeric id") instead of reusing ADAPTER_021, so that callers can distinguish "empty or non-array" errors from "malformed chain element" errors; update any tests or error handling that expect ADAPTER_021 accordingly.
264-277:getSortedChainIdentifiersreuses the wrong error message for an invalid chain element.Line 272 throws
"ADAPTER_021: chains must be a non-empty array"when a chain object within a non-empty array lacks a numericid. The failure mode is a malformed chain element, not an empty array — the same code and message used at Line 265-267 for a genuinely empty array. This makes the two distinct failure modes indistinguishable in logs.♻️ Proposed fix
return chains .map((chain) => { if (!chain || typeof chain.id !== "number") { - throw new AdapterError("ADAPTER_021: chains must be a non-empty array"); + throw new AdapterError("ADAPTER_022: chains contains an invalid chain object (missing or non-numeric id)"); } return `${chain.id}`; })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/providers/AppKitAdapterManager.ts` around lines 264 - 277, The function getSortedChainIdentifiers currently rethrows ADAPTER_021 for both an empty/non-array chains input and for invalid individual chain elements; change the error thrown inside the map validation to a distinct AdapterError (e.g. ADAPTER_022) with a clear message like "ADAPTER_022: chain elements must be objects with a numeric id" to distinguish the malformed-element case from the non-empty-array check, keeping the initial Array.isArray/chains.length check as-is and only modifying the throw inside the map callback that checks !chain || typeof chain.id !== "number".components/layout/sidebar/WebSidebarNav.tsx (2)
36-42:isMobileprop is accepted but unused.The
_isMobileparameter (prefixed with_to suppress lint warnings) is destructured with a default value but never referenced in the component body. If it's a placeholder for upcoming work, consider adding a brief comment; otherwise, remove it to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/layout/sidebar/WebSidebarNav.tsx` around lines 36 - 42, The isMobile prop is destructured as _isMobile in the WebSidebarNav forwardRef component but never used; either remove it from the WebSidebarNavProps destructuring and the parameter list to avoid dead code, or keep it and add a brief explanatory comment above the destructuring (e.g., "// reserved for future mobile behavior") and reference _isMobile where intended; update the WebSidebarNavProps type and the forwardRef signature accordingly (symbols: WebSidebarNav, WebSidebarNavProps, _isMobile).
75-80: No-opuseMemo—sectionsis just a pass-through ofallSections.
useSidebarSectionsalready returns a memoized value. Wrapping it in anotheruseMemo(() => allSections, [allSections])adds no benefit — it returns the same reference whenever the dependency changes, which is exactly whenallSectionsitself changes. You can simply useallSectionsdirectly (or keep the rename inline):♻️ Simplification
- const allSections = useSidebarSections( + const sections = useSidebarSections( appWalletsSupported, capacitor.isIos, country ); - const sections = useMemo(() => allSections, [allSections]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/layout/sidebar/WebSidebarNav.tsx` around lines 75 - 80, The variable sections is a no-op wrapper around the already-memoized value from useSidebarSections; remove the redundant useMemo and use the returned value directly (replace the useMemo call that creates sections with direct use of allSections or rename allSections to sections). Update any references to sections to use the original return (allSections) or the new direct variable, and delete the import/use of useMemo if it becomes unused. Ensure you adjust references so there are no remaining unused variables (allSections or sections) and run a type check.components/waves/memes/traits/NumberTrait.tsx (1)
21-24: Stale JSDoc comment.The comment says "Without min/max constraints," but the component clearly uses
min/maxas required props and enforces them in multiple handlers. Consider updating or removing this misleading doc.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/traits/NumberTrait.tsx` around lines 21 - 24, The JSDoc for NumberTrait is stale/misleading: it claims "Without min/max constraints" while the NumberTrait component and its handlers (props min, max, onChange, validateValue, handleInputBlur, handleArrowChange) treat min and max as required and enforce them; update or remove the JSDoc to reflect that min and max are required and enforced (or document the exact behavior and constraints), and ensure the comment references the prop contract for NumberTrait rather than claiming absence of min/max.components/header/useChainSwitcher.ts (1)
14-35:switchToNextChainandcurrentChainNameare recomputed every render.Both values are re-created on each render. Since
switchToNextChainis passed as a click handler to child components, this could cause unnecessary re-renders in consumers that rely on referential equality (e.g.,React.memo'd buttons). Consider wrapping inuseCallback/useMemo:♻️ Suggested memoization
+import { useCallback, useMemo } from "react"; import { useChainId, useChains, useSwitchChain } from "wagmi"; // ... - const currentChainName = - chains.find((chain) => chain.id === chainId)?.name ?? chainId.toString(); + const currentChainName = useMemo( + () => chains.find((chain) => chain.id === chainId)?.name ?? chainId.toString(), + [chains, chainId] + ); - const switchToNextChain = (): boolean => { + const switchToNextChain = useCallback((): boolean => { // ... existing body ... - }; + }, [chains, chainId, switchChain]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/header/useChainSwitcher.ts` around lines 14 - 35, currentChainName and switchToNextChain are recreated on every render causing potential unnecessary re-renders for consumers relying on referential equality; memoize currentChainName with useMemo (depend on chains and chainId) and wrap switchToNextChain with useCallback (depend on chains, chainId, switchChain) so the computed name and the click handler keep stable identities across renders.components/waves/memes/traits/TextTrait.tsx (1)
96-99: Minor inconsistency:defaultValuedoesn't use.toString().Line 36 normalizes via
.toString()for all comparisons, but line 132'sdefaultValue={(traits[field] as string) ?? ""}skips it. This works because React coercesdefaultValueto a string for<input type="text">, but for consistency with the rest of the file, consider:- defaultValue={(traits[field] as string) ?? ""} + defaultValue={currentTraitValue}This also avoids computing the same value twice.
Also applies to: 132-132
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/memes/traits/TextTrait.tsx` around lines 96 - 99, The input's defaultValue in TextTrait is inconsistent: it sets defaultValue={(traits[field] as string) ?? ""} while elsewhere you normalize with .toString(); update the input's defaultValue to use the same normalization (e.g., (traits[field] as any)?.toString() ?? "") and reuse the already-computed normalized value (the variable used for comparisons like currentInputValue/isFieldFilled) to avoid duplicate computation.components/the-memes/TheMemesMint.tsx (1)
33-33: Testnet contract support is intentional; consider adding user warnings.The
useDropForgeMintingConfig()hook is explicitly designed to returnDROP_FORGE_TESTNET_CONTRACTwhen a user is connected to Sepolia andMEMES_CONTRACTfor other chains. This testnet fallback is part of the Drop Forge workflow and appears intentional. However, there are no upstream chain guards inapp/the-memes/mint/page.tsxand no user-facing warnings about the contract/chain being used. A user connected to Sepolia will silently attempt to mint from the testnet contract, which could cause confusion. Consider adding either a chain validation guard upstream or a UI warning when testnet contracts are detected.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/the-memes/TheMemesMint.tsx` at line 33, The hook useDropForgeMintingConfig can return the testnet fallback (DROP_FORGE_TESTNET_CONTRACT) when on Sepolia, so add a guard or warning: in the mint page component that renders TheMemesMint (the app/the-memes/mint page) check the returned chain/contract from useDropForgeMintingConfig and either block the mint flow for unsupported chains or surface a clear UI banner/modal when the contract equals DROP_FORGE_TESTNET_CONTRACT or chain indicates Sepolia; update TheMemesMint component to render the banner and disable the mint button (or redirect to a supported chain) when the testnet contract is detected, and ensure the check uses the same identifiers (useDropForgeMintingConfig, DROP_FORGE_TESTNET_CONTRACT, MEMES_CONTRACT) so behavior stays consistent.components/drop-forge/DropForgeClaimsListPageClient.tsx (1)
33-90: Deduplicate media-format helpers into a shared module.Lines 33-90 re-implement logic that already exists in
components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts(normalize/extension/image/animation/media label helpers). This is likely to drift.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/DropForgeClaimsListPageClient.tsx` around lines 33 - 90, This file duplicates media-format helper logic already implemented in dropForgeLaunchClaimPageClient.helpers.ts; remove the local implementations of normalizeFormat, getUrlExtension, getImageFormat, getAnimationInfo and getMediaTypeLabel and import the corresponding helpers from the shared module (or re-export/align names there if necessary), then update any local references to call the imported functions so only one canonical implementation remains and the helper code doesn't drift.components/drop-forge/drop-forge-status.helpers.ts (1)
60-95: Make context explicit instead of inferring fromundefinedvsnull.Line 60 uses
manifoldClaim === undefinedto infer craft context. That API contract is easy to break and can flippublishedvspending_initializationunexpectedly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/drop-forge-status.helpers.ts` around lines 60 - 95, The code infers craft context by checking manifoldClaim === undefined which is brittle; change the logic to accept and use an explicit boolean flag (e.g., isCraftContext) passed into the helper instead of relying on manifoldClaim's undefined/null shape, then replace the current local isCraftContext assignment with that parameter and update all uses (the ternary that returns "published" vs "pending_initialization") to consult the explicit isCraftContext flag; ensure callers that invoke the helper are updated to provide the correct boolean.openapi.yaml (1)
8095-8097: ReuseApiEthereumAddressin new wallet/address fields for consistent validation.You introduced
ApiEthereumAddressat Line 8095, butMintingClaimsProofsByAddressEntry.address(Line 11656) andPhaseAirdrop.wallet(Line 11845) are still plain strings.Proposed fix
MintingClaimsProofsByAddressEntry: type: object @@ properties: address: - type: string + $ref: "#/components/schemas/ApiEthereumAddress" description: Wallet address @@ PhaseAirdrop: type: object @@ properties: wallet: - type: string + $ref: "#/components/schemas/ApiEthereumAddress" amount: type: integer format: int64Also applies to: 11656-11658, 11845-11846
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@openapi.yaml` around lines 8095 - 8097, Replace the plain string type for Ethereum addresses with the new reusable schema ApiEthereumAddress: update the MintingClaimsProofsByAddressEntry.address and PhaseAirdrop.wallet fields to reference ApiEthereumAddress instead of type: string so they inherit the ^0x[a-fA-F0-9]{40}$ pattern and ensure consistent validation across the spec.__tests__/components/header/user/proxy/HeaderUserProxyDropdown.test.tsx (1)
8-10: Mock setup is correct but no test exercises the chain-switching UI.The
useChainSwitchermock is properly structured. However, sincechains: []means the "Switch Chain" button never renders, there's no test coverage for the new chain-switching feature added toHeaderUserProxyDropdown. Consider adding a test that provides multiple chains and verifies the button appears and callsswitchToNextChain.Also applies to: 33-37
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@__tests__/components/header/user/proxy/HeaderUserProxyDropdown.test.tsx` around lines 8 - 10, The test currently mocks useChainSwitcher but returns chains: [] so the "Switch Chain" button never renders; update the mock in HeaderUserProxyDropdown.test.tsx to return a non-empty chains array (e.g., two chain objects) and a jest.fn() for switchToNextChain, render HeaderUserProxyDropdown, assert that the "Switch Chain" button (or its label) is present, simulate a click on that button, and verify switchToNextChain was called; reference the useChainSwitcher mock, the HeaderUserProxyDropdown component, and the switchToNextChain mock when making these changes.services/api/memes-minting-claims-api.ts (1)
198-211:uploadClaimMediaignores_claimIdand_field— the upload is not associated with a specific claim or media slot.The function accepts
claimIdandfieldparameters (prefixed with_to suppress unused-variable warnings) but passes neither tomultipartUploadCore. The upload goes to a genericdrop-media/multipart-uploadendpoint with no claim context. If the intent is a two-step flow (upload → get URL → patch claim), this works but the unused parameters are misleading and may cause bugs if a future caller assumes the upload is automatically bound to the claim.Either remove the unused parameters or document the two-step intent:
Option A: Remove unused params
-export async function uploadClaimMedia( - _claimId: number, - _field: ClaimMediaField, - file: File -): Promise<string> { +export async function uploadClaimMedia(file: File): Promise<string> { return multipartUploadCore({ file, endpoints: {Option B: Keep params and add JSDoc
+/** + * Uploads media to a generic storage endpoint and returns the hosted URL. + * The caller is responsible for patching the claim with the returned URL + * via `patchClaim(claimId, { [field]: url })`. + */ export async function uploadClaimMedia( _claimId: number, _field: ClaimMediaField, file: File ): Promise<string> {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@services/api/memes-minting-claims-api.ts` around lines 198 - 211, The uploadClaimMedia function currently accepts _claimId and _field but never uses them (it just calls multipartUploadCore with generic endpoints); remove the unused parameters from the uploadClaimMedia signature (and update any callers) so the function becomes uploadClaimMedia(file: File): Promise<string>, or alternatively keep the parameters but add a clear JSDoc to uploadClaimMedia stating this is a two-step flow (upload returns a media URL/id which must then be patched onto a claim) and/or pass claimId/field through to multipartUploadCore and backend endpoints if you intend the upload to be bound to a claim. Ensure references to multipartUploadCore and the endpoint names ("drop-media/multipart-upload" etc.) are updated accordingly.components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts (2)
234-243:isVideoUrlcan false-positive on URLs containing extensions mid-path or in query strings.
lower.includes(".mp4")matches URLs likehttps://example.com/.mp4-proxy/image.pngor?file=test.mp4&type=thumb. Consider anchoring to the path's trailing extension:Proposed fix
export function isVideoUrl(url: string | null | undefined): boolean { if (!url) return false; - const lower = url.toLowerCase(); - return ( - lower.includes(".mp4") || - lower.includes(".webm") || - lower.includes(".mov") || - lower.includes(".m4v") - ); + const ext = getUrlExtension(url); + return ext === "mp4" || ext === "webm" || ext === "mov" || ext === "m4v"; }This also reuses the already-defined
getUrlExtensionhelper for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts` around lines 234 - 243, isVideoUrl currently does a substring check which false-positives on extensions appearing in query strings or path segments; change it to use the existing getUrlExtension helper to extract the real file extension from the URL and compare that extension (lowercased) against the allowed set ("mp4","webm","mov","m4v"), returning false for null/empty input. Update the isVideoUrl implementation to call getUrlExtension(url) and perform a strict equality check against the set rather than using includes.
15-20:MintingClaimsRootItemWithAliasesre-declares fields already present on the base type.
MintingClaimsRootItemalready definesaddresses_count: numberandtotal_spots: numberas required. The intersection adds them again as optional, which means the intersection type silently resolves to the required variant for those two keys. This is benign at runtime but misleading — readers may thinkaddresses_countandtotal_spotscan be absent on this type when in fact the base type already guarantees them.Consider declaring only the truly additional aliases:
Proposed fix
-type MintingClaimsRootItemWithAliases = MintingClaimsRootItem & { - addresses_count?: number; - addresses?: number; - total_spots?: number; - total?: number; -}; +type MintingClaimsRootItemWithAliases = MintingClaimsRootItem & { + addresses?: number; + total?: number; +};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/dropForgeLaunchClaimPageClient.helpers.ts` around lines 15 - 20, The intersection type MintingClaimsRootItemWithAliases re-declares existing required fields from MintingClaimsRootItem (addresses_count and total_spots) making the alias type misleading; update MintingClaimsRootItemWithAliases to only add the actual alias properties (e.g. keep addresses?: number and total?: number) and remove the redundant addresses_count?: number and total_spots?: number declarations so the type reads MintingClaimsRootItem & { addresses?: number; total?: number; }.
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
Signed-off-by: prxt6529 <prxt@6529.io>
|
2 participants
Summary by CodeRabbit
New Features
Bug Fixes