Conversation
…tion size. 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 on-chain fetching awareness with a new "checking_onchain" primary status, refactors status derivation, introduces helpers and state for computing/clamping research target edition sizes (default 310), threads fetching and size limits through launch client/view, and adds tests covering these behaviors. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as "DropForge UI"
participant Hook as "useDropForgeManifoldClaim"
participant Onchain as "Manifold / Onchain API"
participant Helper as "getClaimPrimaryStatus"
participant Pill as "StatusPill"
UI->>Hook: request manifold claim (claimId)
Hook->>Onchain: fetch on-chain claim state
Onchain-->>Hook: returns { manifoldClaim, isFetching }
UI->>Helper: compute primary status (claim, manifoldClaim, isManifoldClaimFetching)
Helper-->>UI: returns primaryStatus (checking_onchain / publishing / published / ...)
UI->>Pill: render status (showLoader when primaryStatus.key == "publishing" or "checking_onchain")
Pill-->>UI: display pill with tone/label and loader state
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/drop-forge/launch/DropForgeLaunchClaimPageClient.tsx`:
- Around line 974-981: The code computes researchAirdropCount from
cappedResearchTargetEditionSize which only clamps against claim.edition_size via
clampResearchTargetEditionSize, so if the DB edition_size is raised before the
on‑chain manifoldClaim.totalMax is updated we may attempt to airdrop past the
on‑chain ceiling and revert; update the clamp logic and any places using
claim.edition_size (including cappedResearchTargetEditionSize, the new target
change handler, the computation of researchAirdropCount, and calls like
runResearchAirdropWrite()/airdrop) to also respect manifoldClaim.totalMax (use
the minimum of claim.edition_size and manifoldClaim.totalMax when computing
target/ceilings) so that all airdrop counts and validations are bounded by the
live on‑chain ceiling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6d4fc65a-91d7-43d3-a92c-a2f3446c7c29
📒 Files selected for processing (7)
__tests__/components/drop-forge/drop-forge-status.helpers.test.ts__tests__/components/drop-forge/launch/drop-forge-launch-claim-page-client.helpers.test.tscomponents/drop-forge/DropForgeClaimsListPageClient.tsxcomponents/drop-forge/drop-forge-status.helpers.tscomponents/drop-forge/launch/DropForgeLaunchClaimPageClient.tsxcomponents/drop-forge/launch/DropForgeLaunchClaimPageClient.view.tsxcomponents/drop-forge/launch/drop-forge-launch-claim-page-client.helpers.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
components/drop-forge/launch/DropForgeLaunchClaimPageClient.tsx (1)
349-352:⚠️ Potential issue | 🟠 MajorClamp the research target to the lower of the DB and on-chain caps.
getResearchTargetEditionSizeLimit()still treatsmanifoldClaim.totalMaxas an override instead of a cap (components/drop-forge/launch/drop-forge-launch-claim-page-client.helpers.ts:179-194). Across these call sites, that means the rendered value, input max, andresearchAirdropCountcan exceedclaim.edition_sizewhenever the on-chain ceiling is higher than the DB cap. Please bound this with the minimum of the two limits so research airdrops stay inside both ceilings.Also applies to: 986-994, 1488-1517, 1681-1682
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drop-forge/launch/DropForgeLaunchClaimPageClient.tsx` around lines 349 - 352, getResearchTargetEditionSizeLimit is treating manifoldClaim.totalMax as an override instead of a cap, allowing researchTargetEditionSizeLimit (and downstream uses like researchAirdropCount, input max, and rendered value) to exceed claim.edition_size when on-chain ceiling is higher; update the logic where researchTargetEditionSizeLimit is computed (and inside getResearchTargetEditionSizeLimit) to use the minimum of claim?.edition_size and manifoldClaim?.totalMax as the effective cap, ensuring all call sites (e.g., variables named researchTargetEditionSizeLimit, researchAirdropCount and related input max handling) clamp values to Math.min(claim.edition_size, manifoldClaim.totalMax) before rendering or computing counts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/drop-forge/launch/DropForgeLaunchClaimPageClient.tsx`:
- Around line 371-394: Collapse the reset and sync into one useEffect and only
update syncedResearchTargetClaimIdRef/current and call
setResearchTargetEditionSize from that effect when the claim data is known to
belong to the current claimId (e.g. check claim?.id === claimId or maintain a
loadedClaimId and verify it matches). Specifically, replace the two separate
effects with a single effect watching [claimId, claim, manifoldClaim?.totalMax]
that first clears state when claimId changes (set
syncedResearchTargetClaimIdRef.current = null and
setResearchTargetEditionSize(getDefaultResearchTargetEditionSize(null))) and
then, only if claim is non-null and its id matches claimId, set
syncedResearchTargetClaimIdRef.current = claimId and compute the default via
getDefaultResearchTargetEditionSize(claim.edition_size,
manifoldClaim?.totalMax); otherwise avoid marking synced for the new id so
subsequent real claim data can initialize or clamp via
clampResearchTargetEditionSize safely.
---
Duplicate comments:
In `@components/drop-forge/launch/DropForgeLaunchClaimPageClient.tsx`:
- Around line 349-352: getResearchTargetEditionSizeLimit is treating
manifoldClaim.totalMax as an override instead of a cap, allowing
researchTargetEditionSizeLimit (and downstream uses like researchAirdropCount,
input max, and rendered value) to exceed claim.edition_size when on-chain
ceiling is higher; update the logic where researchTargetEditionSizeLimit is
computed (and inside getResearchTargetEditionSizeLimit) to use the minimum of
claim?.edition_size and manifoldClaim?.totalMax as the effective cap, ensuring
all call sites (e.g., variables named researchTargetEditionSizeLimit,
researchAirdropCount and related input max handling) clamp values to
Math.min(claim.edition_size, manifoldClaim.totalMax) before rendering or
computing counts.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74d1ef10-d605-4dfe-a6f9-fa9c02ea7494
📒 Files selected for processing (3)
__tests__/components/drop-forge/launch/drop-forge-launch-claim-page-client.helpers.test.tscomponents/drop-forge/launch/DropForgeLaunchClaimPageClient.tsxcomponents/drop-forge/launch/drop-forge-launch-claim-page-client.helpers.ts
✅ Files skipped from review due to trivial changes (1)
- tests/components/drop-forge/launch/drop-forge-launch-claim-page-client.helpers.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- components/drop-forge/launch/drop-forge-launch-claim-page-client.helpers.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/drop-forge/drop-forge-status.helpers.ts (1)
93-117:⚠️ Potential issue | 🟠 MajorAdd
!isCraftContextguard to the launch-only missing-info state.The craft
publishedbranch (lines 94–109) only runs whenmissingLaunchInfois false, but lines 111–117 do not checkisCraftContext. SinceisMissingRequiredLaunchInfochecks for launch-specific fields likeedition_sizeandseason, craft claims lacking these fields will return true and hit the unguarded branch, emittingpending_initialization_missing_info—a state not expected in the craft flow (DropForgeCraftClaimPageClient lines 1668–1725 only handles "draft", "publishing", and "published").Suggested fix
- if (!initializedOnchain && missingLaunchInfo) { + if (!isCraftContext && !initializedOnchain && missingLaunchInfo) { return { key: "pending_initialization_missing_info", label: "Pending Initialization — Missing Info", tone: "pending", reason: "Not onchain yet. Required launch fields are missing",🤖 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 93 - 117, The branch that returns the "pending_initialization_missing_info" status should be guarded so it does not run for craft claims; modify the condition that currently reads if (!initializedOnchain && missingLaunchInfo) to also require !isCraftContext (i.e., if (!initializedOnchain && missingLaunchInfo && !isCraftContext)), so craft flow (handled by DropForgeCraftClaimPageClient) will continue to use its "published"/craft-only branches and won't receive the launch-only missing-info state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@components/drop-forge/drop-forge-status.helpers.ts`:
- Around line 93-117: The branch that returns the
"pending_initialization_missing_info" status should be guarded so it does not
run for craft claims; modify the condition that currently reads if
(!initializedOnchain && missingLaunchInfo) to also require !isCraftContext
(i.e., if (!initializedOnchain && missingLaunchInfo && !isCraftContext)), so
craft flow (handled by DropForgeCraftClaimPageClient) will continue to use its
"published"/craft-only branches and won't receive the launch-only missing-info
state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ccf687b5-bee9-499f-bfe1-05e2be2d9d50
📒 Files selected for processing (2)
__tests__/components/drop-forge/drop-forge-status.helpers.test.tscomponents/drop-forge/drop-forge-status.helpers.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/components/drop-forge/drop-forge-status.helpers.test.ts
Signed-off-by: prxt6529 <prxt@6529.io>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
components/drop-forge/drop-forge-status.helpers.ts (1)
158-163: The API design conflates two distinct states usingundefined: "craft context" and "query loading."Currently, all non-craft callers (DropForgeClaimsListPageClient and DropForgeLaunchClaimPageClient) properly normalize undefined to null with the
manifoldClaim ?? nullpattern. However, this normalization is implicit and not enforced by the API contract. Line 170'sconst isCraftContext = manifoldClaim === undefined;depends on caller discipline—if a non-craft caller omits normalization, the helper would misclassify loading state as craft context, causing the logic at lines 79–91 to skipchecking_onchainand fall back to the craft path instead.While this isn't currently a problem in practice, making context explicit through a dedicated parameter or normalizing the loading state consistently (e.g., always using null for non-craft, never undefined) would eliminate this fragility and prevent future regressions when the API is used elsewhere.
🤖 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 158 - 163, The helper currently treats manifoldClaim === undefined as "craft context", which is fragile; change the API to accept an explicit boolean (e.g., add parameter isCraftContext?: boolean to the function signature) and compute isCraftContext from that parameter instead of checking manifoldClaim for undefined; update all callers (notably DropForgeClaimsListPageClient and DropForgeLaunchClaimPageClient) to pass false (they already normalize manifoldClaim to null) and any craft caller to pass true so the helper no longer conflates loading (undefined) with craft context.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/drop-forge/drop-forge-status.helpers.ts`:
- Line 164: The initialization check uses truthiness which treats instanceId ===
0 as false; update the check used to set initializedOnchain (currently "const
initializedOnchain = !!manifoldClaim?.instanceId") to a nullish check like
"manifoldClaim?.instanceId != null" so zero is treated as a valid initialized
value, and apply the same fix where Boolean(manifoldClaim?.instanceId) is used
(e.g., the DropForgeLaunchClaimPageClient usage) to use a nullish comparison
instead.
---
Nitpick comments:
In `@components/drop-forge/drop-forge-status.helpers.ts`:
- Around line 158-163: The helper currently treats manifoldClaim === undefined
as "craft context", which is fragile; change the API to accept an explicit
boolean (e.g., add parameter isCraftContext?: boolean to the function signature)
and compute isCraftContext from that parameter instead of checking manifoldClaim
for undefined; update all callers (notably DropForgeClaimsListPageClient and
DropForgeLaunchClaimPageClient) to pass false (they already normalize
manifoldClaim to null) and any craft caller to pass true so the helper no longer
conflates loading (undefined) with craft context.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00b02ded-3e1b-4e1b-8be7-e800298057e5
📒 Files selected for processing (2)
__tests__/components/drop-forge/drop-forge-status.helpers.test.tscomponents/drop-forge/drop-forge-status.helpers.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/components/drop-forge/drop-forge-status.helpers.test.ts
|
2 participants
Summary by CodeRabbit
New Features
Improvements
Tests