feat: add structured submission form with draft saving and success state#95
Merged
Benjtalkshow merged 3 commits intoboundlessfi:mainfrom Feb 22, 2026
Merged
Conversation
|
@od-hunter is attempting to deploy a commit to the Threadflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughAdds a structured submission dialog and client form with zod/react-hook-form validation and draft persistence; updates CTAs to open the dialog; extends client API and types and adds a server submit route validating and persisting richer submission fields (explanation, walletAddress, githubUrl, demoUrl, attachments) with explicit error responses. Changes
Sequence DiagramsequenceDiagram
actor User
participant Dialog as SubmissionDialog
participant Form as "react-hook-form/zod"
participant Draft as "LocalStorage Draft"
participant API as "bountiesApi.submit"
participant Server as "/api/bounties/[id]/submit"
participant Store as BountyStore
User->>Dialog: Open dialog
Dialog->>Draft: Check / restore draft
alt Draft found
Draft->>Form: Populate fields
Dialog->>User: Show "draft restored"
end
User->>Form: Fill fields (explanation, urls, attachments, wallet)
User->>Dialog: Click Save Draft
Dialog->>Draft: Persist draft
User->>Dialog: Click Submit
Form->>Form: Validate with submissionFormSchema
alt Validation fails
Form->>User: Show field errors (400)
else Validation passes
Dialog->>API: submit(bountyId, form + contributorId)
API->>Server: POST payload
Server->>Server: Authenticate user, validate payload, check bounty existence/status/type
alt Duplicate exists
Server->>API: 409 Conflict
API->>Dialog: Show duplicate error
else
Server->>Store: addSubmission(enriched submission)
Store->>Server: Return created submission
Server->>API: 200 + submission
API->>Dialog: Success
Dialog->>Draft: Clear draft
Dialog->>Form: Reset form
Dialog->>User: Show success and close
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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: 4
🧹 Nitpick comments (11)
components/bounty-detail/submission-dialog.tsx (3)
75-83: Suppressedreact-hooks/exhaustive-deps— acceptable here but document the intent.Excluding
draftandformfrom the dependency array is intentional: the effect should only fire whenopenchanges. The eslint-disable is pragmatic, but a brief comment explaining why these deps are excluded improves maintainability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/submission-dialog.tsx` around lines 75 - 83, Add an inline comment above the useEffect explaining that the eslint-disable-next-line is intentional because the effect should only run when the dialog `open` prop changes (not when `draft` or `form` mutate), and that `form.reset(draft)` and `setSubmitted(false)` are intentionally tied to `open` transitions; reference the useEffect, `open`, `draft`, `form`, `form.reset`, and `setSubmitted` symbols so future maintainers understand why `react-hooks/exhaustive-deps` was suppressed.
289-294: "Draft restored" notice persists even after user edits the form.The
draftvalue fromuseLocalStorageremains truthy for the entire session (it's only nullified onclearDraft()or submit). This means the "Draft restored from previous session" message stays visible even after the user has modified all fields. Consider tracking whether the draft was actually restored with a separate boolean state that's set once on open.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/submission-dialog.tsx` around lines 289 - 294, The "Draft restored from previous session" message is tied directly to the persistent draft from useLocalStorage (draft) so it stays visible after the user edits; change SubmissionDialog to track a one-time restored flag (e.g., restoredDraftShown or hasRestoredDraft) that is set to true only when the dialog opens and a draft exists, use that boolean to render the message instead of draft, and ensure clearDraft() or onSubmit/close logic clears or ignores the flag appropriately; update the component's state initialization and the code paths around draft, clearDraft(), and the dialog open handler to set/reset this new flag.
70-73:useFieldArraywithas nevercast — fragile workaround for string arrays.
useFieldArrayis designed for arrays of objects (with anidfield). Using it with a flatstring[]requires theas nevercast and may break with futurereact-hook-formupdates. Consider structuring attachments as{ url: string }[]in the schema, which gives proper typing and avoids the cast.💡 Schema and usage change
In
schemas.ts:- attachments: z.array(z.string().url("Must be a valid URL")).optional(), + attachments: z.array(z.object({ url: z.string().url("Must be a valid URL") })).optional(),Then in the dialog,
useFieldArrayworks naturally without casts, and you accessfield.urlin the input.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/submission-dialog.tsx` around lines 70 - 73, The current use of useFieldArray({ name: "attachments" as never }) with a flat string[] is a fragile cast; change the attachments schema/type to an array of objects (e.g., { url: string }[]) and update the form schema accordingly so useFieldArray can be used without "as never". Update the component code that references fields, append, and remove to treat each field as an object (access field.url in inputs and append({ url: value }) / remove(index) as needed) so typing is correct and the cast can be removed.lib/api/bounties.ts (1)
4-4: Inverted dependency:lib/apiimports fromcomponents/.
lib/api/bounties.tsimportsSubmissionFormValuefrom@/components/bounty/forms/schemas. This creates a dependency from the API/data layer into the component/UI layer, which inverts the typical dependency direction. If the schema needs to be shared between the API client, the server route, and the form UI, consider movingsubmissionFormSchemaandSubmissionFormValueto a shared location such aslib/schemas/ortypes/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/api/bounties.ts` at line 4, lib/api/bounties.ts currently imports SubmissionFormValue from the UI layer (components/bounty/forms/schemas), inverting dependencies; move the schema and type (submissionFormSchema and SubmissionFormValue) to a shared module (e.g., lib/schemas or types) and update imports in lib/api/bounties.ts, the server route(s), and the form component to import from that shared location so the API/data layer no longer depends on components.components/bounty-detail/bounty-detail-sidebar-cta.tsx (2)
30-43: Duplicated CTA label logic betweenSidebarCTAandMobileCTA.
ctaLabel()(Line 30) andlabel()(Line 181) are identical. Extract to a shared helper to avoid the duplication.♻️ Suggested extraction
function getCtaLabel(bounty: Bounty): string { if (bounty.status !== "open") return bounty.status === "claimed" ? "Already Claimed" : "Bounty Closed"; switch (bounty.claimingModel) { case "single-claim": return "Claim Bounty"; case "application": return "Apply Now"; case "competition": return "Submit Entry"; case "multi-winner": return "Submit Work"; } }Then use
getCtaLabel(bounty)in both components.Also applies to: 181-194
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 30 - 43, The CTA label logic is duplicated between the ctaLabel() function and label() in the same file; extract this logic into a single helper like getCtaLabel(bounty) that takes the bounty object and returns the correct string (handling non-open status -> "Already Claimed" or "Bounty Closed" and mapping claimingModel values to "Claim Bounty", "Apply Now", "Submit Entry", "Submit Work"), then replace both ctaLabel() and label() callers with calls to getCtaLabel(bounty) to remove duplication.
112-117: BothSidebarCTAandMobileCTArender their ownSubmissionDialog— risk of duplicate dialogs.
MobileCTAis hidden vialg:hiddenCSS (Line 197) whileSidebarCTAlikely appears in a sidebar visible atlg+. Both are always mounted in the DOM, each with independentdialogOpenstate and their ownSubmissionDialoginstance. While only one CTA button is visually accessible at a time, both dialog instances exist in the React tree.Consider lifting the dialog state to a shared parent or rendering a single
SubmissionDialoginstance to avoid potential issues with duplicate draft handling (both reading/writing the samelocalStoragekey).Also applies to: 207-212
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx` around lines 112 - 117, Both SidebarCTA and MobileCTA mount independent SubmissionDialog instances with separate dialogOpen state, causing duplicate dialogs and conflicting localStorage drafts; fix by lifting the dialog state (dialogOpen and setDialogOpen) up to their shared parent (the bounty detail component) and render a single SubmissionDialog there (pass bountyId, bountyTitle, open, and onOpenChange down or omit child dialogs), or alternatively conditionally render SubmissionDialog only once based on viewport breakpoint in the parent; update SidebarCTA and MobileCTA to call a prop handler (e.g., onRequestOpen) instead of owning dialogOpen so only one SubmissionDialog instance manages drafts/localStorage.components/bounty/forms/schemas.ts (3)
78-78: Consider adding wallet address format validation.
walletAddressonly checks for a non-empty string. Given the placeholder hints at Stellar (G...) or EVM (0x...) addresses, consider adding a regex or refinement to validate the format. At minimum, a reasonable.max()would prevent abuse.💡 Example refinement
- walletAddress: z.string().min(1, "Wallet address is required"), + walletAddress: z + .string() + .min(1, "Wallet address is required") + .max(256, "Wallet address is too long") + .refine( + (val) => /^(G[A-Z2-7]{55}|0x[a-fA-F0-9]{40})$/.test(val), + "Must be a valid Stellar or EVM wallet address", + ),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/forms/schemas.ts` at line 78, The walletAddress schema currently only checks non-empty; update the walletAddress validator to enforce format and length by adding a reasonable .max() (e.g., 64) and a refinement or pattern check that accepts common address formats (e.g., EVM hex starting with "0x" and Stellar public keys starting with "G") — implement this on the walletAddress z.string() definition (use .regex(...) or .refine(...) with clear error messages) so invalid formats are rejected.
77-77: Attachments array has no upper bound.A user could add an unlimited number of attachment URLs. Consider capping the array length to prevent payload bloat and potential abuse.
💡 Suggested fix
- attachments: z.array(z.string().url("Must be a valid URL")).optional(), + attachments: z.array(z.string().url("Must be a valid URL")).max(10, "Maximum 10 attachments allowed").optional(),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/forms/schemas.ts` at line 77, The attachments schema currently allows an unlimited array length; update the attachments field (attachments: z.array(...).optional()) to enforce an upper bound (e.g., .max(10, "At most 10 attachments allowed")) so the Zod schema rejects oversized payloads; modify the attachments declaration in the schema file to apply the .max(...) constraint to the z.array(...) validator (and adjust the number if you prefer a different cap).
71-72: Schema accepts empty strings for URL fields — confirm this is handled downstream.
.optional().or(z.literal(""))meansparsed.data.githubUrlcan be"". The API route and dialog both normalize empty strings toundefined(e.g.,githubUrl: data.githubUrl || undefined), so this works end-to-end. Just noting it's a two-step validation pattern — the schema allows""and callers must remember to coerce.An alternative is to use
.transform()to normalize at the schema level:💡 Example with transform
- githubUrl: z.string().url("Must be a valid URL").optional().or(z.literal("")), + githubUrl: z.string().url("Must be a valid URL").optional().or(z.literal("")).transform(v => v || undefined),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/bounty/forms/schemas.ts` around lines 71 - 72, The schema currently allows empty strings for githubUrl and demoUrl via `githubUrl` and `demoUrl` fields which use `.optional().or(z.literal(""))`, forcing downstream code to coerce `""` to undefined; instead update those fields in the schema (the `githubUrl` and `demoUrl` definitions) to normalize empty strings to undefined with a transform (e.g., replace the union/optional pattern with a single string/optional and add `.transform(value => value === "" ? undefined : value)`) so parsing returns `undefined` for empty inputs and callers no longer need to coerce values.app/api/bounties/[id]/submit/route.ts (2)
4-4: Server route also imports fromcomponents/— same dependency inversion.As noted in the
lib/api/bounties.tsreview, importingsubmissionFormSchemafrom@/components/bounty/forms/schemasinto a server API route is an inverted dependency. Moving the schema to a sharedlib/ortypes/location would fix both import sites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bounties/`[id]/submit/route.ts at line 4, submissionFormSchema is imported into a server route (route.ts) from a components path which creates a dependency inversion; move the submissionFormSchema definition out of "@/components/bounty/forms/schemas" into a shared location (e.g., lib/schemas or types/bounty) and update all consumers (notably the server route handler in app/api/bounties/[id]/submit/route.ts and the client helper in lib/api/bounties.ts) to import submissionFormSchema from the new shared module; ensure the exported symbol name remains submissionFormSchema and adjust any relative import paths and build/type exports accordingly.
39-50:allowedModelsguard is currently a no-op.The allowlist contains all four claiming model values (
single-claim,competition,multi-winner,application), which matches the fullclaimingModelSchemaenum. This means the check will never fail. If the intent is future-proofing for new models, add a comment. Otherwise, remove the dead check to reduce noise.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bounties/`[id]/submit/route.ts around lines 39 - 50, The allowedModels guard (the allowedModels array and the includes check against bounty.claimingModel) is effectively a no-op because it lists all values from the claimingModel enum; remove the dead check to reduce noise by deleting the allowedModels array and the if-block that returns the 400 response, or if you intended this as future-proofing, keep the guard but replace it with a clear explanatory comment above it (e.g., “// Keep this allowlist to explicitly block new claiming models until handled”) so the intent is explicit.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/bounties/`[id]/submit/route.ts:
- Around line 34-50: The route currently validates existence and claiming model
(BountyStore.getBountyById, allowedModels and bounty.claimingModel) but never
checks bounty.status, so submissions can be made to closed/claimed bounties; add
a guard after fetching the bounty that verifies bounty.status === "open" (or an
allowed set of statuses) and return a 400/409 JSON error (e.g., "Bounty is not
open for submissions") if the status is not open, keeping this check before any
further validation or processing.
In `@components/bounty-detail/submission-dialog.tsx`:
- Around line 114-117: The timeout started in the submission dialog (the
setTimeout that calls onOpenChange(false) and setSubmitted(false)) needs cleanup
to avoid firing after unmount or dialog close; store the timeout ID in a ref
(e.g., timeoutRef), set it when calling setTimeout, and clear it with
clearTimeout(timeoutRef.current) in a useEffect cleanup or whenever the dialog
is closed (and reset the ref to null) so the pending callback cannot run on a
stale closure.
- Line 104: Replace the hardcoded contributorId ("current-user") with the
authenticated user's ID by importing and calling authClient.useSession() inside
the submission handler in submission-dialog.tsx, then set contributorId to
session?.user?.id (or equivalent user id property); also handle the no-session
case (disable submit or show error) so submissions aren't sent with a missing
ID. Ensure you update the payload construction where contributorId is set and
remove the placeholder string.
In `@types/participation.ts`:
- Around line 23-26: The Submission interface defines duplicate fields content
and explanation; either remove content and update all references (including the
submit handler that currently sets content = explanation) to use explanation
only, or mark content as deprecated and add a clear comment/JSdoc on the
Submission type while keeping the submit handler mapping for backward
compatibility (e.g., preserve the assignment in the function that creates
Submission but add a TODO/@deprecated on content). Locate the Submission
interface symbol and the submit handler function that assigns content =
explanation (the handler in the API submit route) and apply one of these two
fixes consistently across usages.
---
Nitpick comments:
In `@app/api/bounties/`[id]/submit/route.ts:
- Line 4: submissionFormSchema is imported into a server route (route.ts) from a
components path which creates a dependency inversion; move the
submissionFormSchema definition out of "@/components/bounty/forms/schemas" into
a shared location (e.g., lib/schemas or types/bounty) and update all consumers
(notably the server route handler in app/api/bounties/[id]/submit/route.ts and
the client helper in lib/api/bounties.ts) to import submissionFormSchema from
the new shared module; ensure the exported symbol name remains
submissionFormSchema and adjust any relative import paths and build/type exports
accordingly.
- Around line 39-50: The allowedModels guard (the allowedModels array and the
includes check against bounty.claimingModel) is effectively a no-op because it
lists all values from the claimingModel enum; remove the dead check to reduce
noise by deleting the allowedModels array and the if-block that returns the 400
response, or if you intended this as future-proofing, keep the guard but replace
it with a clear explanatory comment above it (e.g., “// Keep this allowlist to
explicitly block new claiming models until handled”) so the intent is explicit.
In `@components/bounty-detail/bounty-detail-sidebar-cta.tsx`:
- Around line 30-43: The CTA label logic is duplicated between the ctaLabel()
function and label() in the same file; extract this logic into a single helper
like getCtaLabel(bounty) that takes the bounty object and returns the correct
string (handling non-open status -> "Already Claimed" or "Bounty Closed" and
mapping claimingModel values to "Claim Bounty", "Apply Now", "Submit Entry",
"Submit Work"), then replace both ctaLabel() and label() callers with calls to
getCtaLabel(bounty) to remove duplication.
- Around line 112-117: Both SidebarCTA and MobileCTA mount independent
SubmissionDialog instances with separate dialogOpen state, causing duplicate
dialogs and conflicting localStorage drafts; fix by lifting the dialog state
(dialogOpen and setDialogOpen) up to their shared parent (the bounty detail
component) and render a single SubmissionDialog there (pass bountyId,
bountyTitle, open, and onOpenChange down or omit child dialogs), or
alternatively conditionally render SubmissionDialog only once based on viewport
breakpoint in the parent; update SidebarCTA and MobileCTA to call a prop handler
(e.g., onRequestOpen) instead of owning dialogOpen so only one SubmissionDialog
instance manages drafts/localStorage.
In `@components/bounty-detail/submission-dialog.tsx`:
- Around line 75-83: Add an inline comment above the useEffect explaining that
the eslint-disable-next-line is intentional because the effect should only run
when the dialog `open` prop changes (not when `draft` or `form` mutate), and
that `form.reset(draft)` and `setSubmitted(false)` are intentionally tied to
`open` transitions; reference the useEffect, `open`, `draft`, `form`,
`form.reset`, and `setSubmitted` symbols so future maintainers understand why
`react-hooks/exhaustive-deps` was suppressed.
- Around line 289-294: The "Draft restored from previous session" message is
tied directly to the persistent draft from useLocalStorage (draft) so it stays
visible after the user edits; change SubmissionDialog to track a one-time
restored flag (e.g., restoredDraftShown or hasRestoredDraft) that is set to true
only when the dialog opens and a draft exists, use that boolean to render the
message instead of draft, and ensure clearDraft() or onSubmit/close logic clears
or ignores the flag appropriately; update the component's state initialization
and the code paths around draft, clearDraft(), and the dialog open handler to
set/reset this new flag.
- Around line 70-73: The current use of useFieldArray({ name: "attachments" as
never }) with a flat string[] is a fragile cast; change the attachments
schema/type to an array of objects (e.g., { url: string }[]) and update the form
schema accordingly so useFieldArray can be used without "as never". Update the
component code that references fields, append, and remove to treat each field as
an object (access field.url in inputs and append({ url: value }) / remove(index)
as needed) so typing is correct and the cast can be removed.
In `@components/bounty/forms/schemas.ts`:
- Line 78: The walletAddress schema currently only checks non-empty; update the
walletAddress validator to enforce format and length by adding a reasonable
.max() (e.g., 64) and a refinement or pattern check that accepts common address
formats (e.g., EVM hex starting with "0x" and Stellar public keys starting with
"G") — implement this on the walletAddress z.string() definition (use
.regex(...) or .refine(...) with clear error messages) so invalid formats are
rejected.
- Line 77: The attachments schema currently allows an unlimited array length;
update the attachments field (attachments: z.array(...).optional()) to enforce
an upper bound (e.g., .max(10, "At most 10 attachments allowed")) so the Zod
schema rejects oversized payloads; modify the attachments declaration in the
schema file to apply the .max(...) constraint to the z.array(...) validator (and
adjust the number if you prefer a different cap).
- Around line 71-72: The schema currently allows empty strings for githubUrl and
demoUrl via `githubUrl` and `demoUrl` fields which use
`.optional().or(z.literal(""))`, forcing downstream code to coerce `""` to
undefined; instead update those fields in the schema (the `githubUrl` and
`demoUrl` definitions) to normalize empty strings to undefined with a transform
(e.g., replace the union/optional pattern with a single string/optional and add
`.transform(value => value === "" ? undefined : value)`) so parsing returns
`undefined` for empty inputs and callers no longer need to coerce values.
In `@lib/api/bounties.ts`:
- Line 4: lib/api/bounties.ts currently imports SubmissionFormValue from the UI
layer (components/bounty/forms/schemas), inverting dependencies; move the schema
and type (submissionFormSchema and SubmissionFormValue) to a shared module
(e.g., lib/schemas or types) and update imports in lib/api/bounties.ts, the
server route(s), and the form component to import from that shared location so
the API/data layer no longer depends on components.
Contributor
|
Hello @od-hunter, fix the coderabbit review. |
Contributor
|
Also, the wallet address input should be disabled. It will be prefilled, since we’re abstracting the user’s wallet. |
Contributor
|
Kindly fix conflict too |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
app/api/bounties/[id]/submit/route.ts (2)
90-95: Swallowed error — thecatchblock discards the exception without logging.When something unexpected goes wrong, the 500 response provides no diagnostics. At minimum, log the error for server-side observability.
Proposed fix
- } catch { + } catch (error) { + console.error("Submission failed:", error); return NextResponse.json( { error: "Internal Server Error" }, { status: 500 }, ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bounties/`[id]/submit/route.ts around lines 90 - 95, The catch block in app/api/bounties/[id]/submit/route.ts swallows exceptions; change the anonymous catch to catch (err) and log the error before returning the 500 response (e.g., console.error or the app logger with a contextual message like "Error in submit handler" plus err/err.stack), keeping the same NextResponse.json({ error: "Internal Server Error" }, { status: 500 }) return. Ensure you update the catch in the submit route handler function so errors are recorded for observability.
4-4: Shared validation schema imported from acomponents/path.
submissionFormSchemalives undercomponents/bounty/forms/schemasbut is used by both a client component and this server-side API route. Consider relocating it to a shared location (e.g.,lib/schemas/orlib/validations/) so the dependency direction stays clean.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bounties/`[id]/submit/route.ts at line 4, submissionFormSchema is currently declared under components and used by both a client component and this server API route; move the schema to a shared module (e.g., lib/schemas or lib/validations), export it with the same name (submissionFormSchema), update imports in this route (route.ts) and the client component to import from the new shared path, and ensure the relocated module has no client-only dependencies (DOM/browser APIs) so it can be safely imported server-side.app/profile/[userId]/page.tsx (1)
98-102: Fragile 404 detection via type assertion and string matching.Casting to
{ status?: number; message?: string }and falling back tomessage?.includes("404")is brittle — it depends on the error shape the hook happens to return and on the literal text of error messages. If the hook or underlying fetch library changes its error shape, this silently stops detecting 404s.Consider having
useContributorReputationexpose a typed error (or status code) so the consumer doesn't resort to string parsing.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/profile/`[userId]/page.tsx around lines 98 - 102, The current 404 detection in the error block is brittle because it casts error to a loose shape and string-matches the message; update the contract so useContributorReputation returns a well-typed error/status (or an explicit isNotFound boolean) and change the consumer code (the error branch that defines apiError and isNotFound) to check that typed status field (e.g., error.status === 404) or the explicit flag instead of using message?.includes("404"); locate references to useContributorReputation, the local error variable, apiError, and isNotFound and ensure the hook signature and its callers are updated together so detection relies on a typed status or flag, not string parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/bounties/`[id]/submit/route.ts:
- Around line 14-23: Replace trusting contributorId from the request body with
authentication via getCurrentUser(): call getCurrentUser() (from
lib/server-auth.ts) at the start of the handler, ensure it returns a user and
use its id as the contributor id instead of the body’s contributorId, return a
401/403 NextResponse.json if no authenticated user is found, and remove or
ignore the incoming contributorId to prevent impersonation; optionally, if you
want to enforce client-provided contributorId, compare it with the authenticated
user's id (using the handler's contributorId variable) and return 403 on
mismatch.
In `@app/profile/`[userId]/page.tsx:
- Around line 28-55: The mockHistory useMemo block currently fabricates
completion records (mockHistory) and supplies them to CompletionHistory; remove
this hardcoded mock and instead source real completion history (e.g., call your
completion-history API using the userId prop via fetch/SWR/react-query inside a
hook or component) and pass that fetched array to CompletionHistory, and if the
endpoint is not available or returns empty, return [] or render a proper
empty/coming-soon state; update the useMemo/mockHistory block (and any tests) to
use the real fetch hook (or a new fetchCompletionHistory(userId) function) and
keep MAX_MOCK_HISTORY only for limiting real results if needed.
- Around line 57-84: The claim-status logic in the myClaims useMemo is inverted:
when bounty.status === "claimed" and claimExpiresAt is in the past the code
currently sets status = "in-review"; change this branch so an expired claim is
labeled appropriately (e.g., "expired" or the project’s canonical
forfeited/overdue status) instead of "in-review". Update the status assignment
in the mapping of bountyResponse?.data inside myClaims (referencing
bounty.status, bounty.claimExpiresAt, and the status variable) to return the
correct expired status when claimExpiresAt < new Date(), leaving the other
status mappings (closed -> "completed", default "active") unchanged.
In `@app/transparency/page.tsx`:
- Around line 100-176: When statsError is true the UI still builds statCards
with numeric defaults, causing zeroed metrics to render; update the rendering
logic so the stats grid is not shown during an error (or compute statCards to
show a non-numeric placeholder). Concretely, wrap the section that maps
statCards (or the JSX containing <section> Platform Overview and the grid) with
a conditional that returns early when statsError is true, or modify the
statCards construction to set values to "—" when statsError is true (use the
existing symbols statCards, statsError, statsLoading, and StatCard to locate the
code); ensure the Try Again button and Alert remain visible and that isLoading
(statsLoading) still drives the StatCard loading state when applicable.
- Around line 100-176: The stat grid shows zero-value defaults while an error
banner is displayed because statCards uses fallback values when stats is null,
so when statsError is true (and statsLoading is false) users see zeros; update
the statCards construction (or before rendering) to check statsError and replace
fallback values with a neutral placeholder (e.g., "—" or similar) or set
StatCard into a loading/errored state by passing an error prop; specifically
modify the statCards value expressions (used by StatCard) to use statsError ?
"—" : (stats ? ... : ...), or alternately hide/gate the entire Platform Overview
section when statsError is true so the Alert is the sole UI shown.
- Around line 27-30: The type annotation uses React.ElementType without
importing React, which breaks under strict TS module settings; fix by adding an
explicit type import from React and updating the annotation: import type {
ElementType } from 'react' and replace React.ElementType with ElementType for
the icon property (or alternatively import type React and keep
React.ElementType); update the interface/type that declares title, value, icon,
isLoading accordingly.
---
Nitpick comments:
In `@app/api/bounties/`[id]/submit/route.ts:
- Around line 90-95: The catch block in app/api/bounties/[id]/submit/route.ts
swallows exceptions; change the anonymous catch to catch (err) and log the error
before returning the 500 response (e.g., console.error or the app logger with a
contextual message like "Error in submit handler" plus err/err.stack), keeping
the same NextResponse.json({ error: "Internal Server Error" }, { status: 500 })
return. Ensure you update the catch in the submit route handler function so
errors are recorded for observability.
- Line 4: submissionFormSchema is currently declared under components and used
by both a client component and this server API route; move the schema to a
shared module (e.g., lib/schemas or lib/validations), export it with the same
name (submissionFormSchema), update imports in this route (route.ts) and the
client component to import from the new shared path, and ensure the relocated
module has no client-only dependencies (DOM/browser APIs) so it can be safely
imported server-side.
In `@app/profile/`[userId]/page.tsx:
- Around line 98-102: The current 404 detection in the error block is brittle
because it casts error to a loose shape and string-matches the message; update
the contract so useContributorReputation returns a well-typed error/status (or
an explicit isNotFound boolean) and change the consumer code (the error branch
that defines apiError and isNotFound) to check that typed status field (e.g.,
error.status === 404) or the explicit flag instead of using
message?.includes("404"); locate references to useContributorReputation, the
local error variable, apiError, and isNotFound and ensure the hook signature and
its callers are updated together so detection relies on a typed status or flag,
not string parsing.
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
app/api/bounties/[id]/submit/route.ts (1)
46-51: MoveallowedModelsto module scope.The array is static and rebuilt on every request invocation. Hoisting it to module level is a trivial, zero-risk improvement.
♻️ Proposed refactor
+ const ALLOWED_CLAIMING_MODELS = [ + "single-claim", + "competition", + "multi-winner", + "application", + ] as const; + const generateId = () => crypto.randomUUID(); // ... inside POST handler: - const allowedModels = [ - "single-claim", - "competition", - "multi-winner", - "application", - ]; - if (!allowedModels.includes(bounty.claimingModel)) { + if (!ALLOWED_CLAIMING_MODELS.includes(bounty.claimingModel)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/bounties/`[id]/submit/route.ts around lines 46 - 51, The allowedModels array is currently created per-request; hoist it to module scope by moving the const allowedModels = [...] declaration out of the request handler in route.ts (so it becomes a top-level module constant) and update any references inside the handler (e.g., validation logic that uses allowedModels) to use this module-level variable; keep the identifier allowedModels unchanged to minimize code changes.components/reputation/my-claims.tsx (1)
62-129: Consider filtering out empty sections to reduce visual noise.All four sections (including "Expired") are always rendered, each with "No claims in this section." / "No opportunities yet." even if the user has zero claims. On a clean profile this produces four empty cards simultaneously.
♻️ Suggested optional filter
- {getClaimsBySection(claims).map(({ section, claims: sectionClaims }) => { + {getClaimsBySection(claims).filter(({ claims: sectionClaims }) => sectionClaims.length > 0).map(({ section, claims: sectionClaims }) => {Or keep the empty-state cards but only for the sections that are contextually meaningful (e.g., always show "Active Claims", hide "Expired" when empty).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/reputation/my-claims.tsx` around lines 62 - 129, The UI currently renders every section returned by getClaimsBySection(claims) even when sectionClaims is empty; update the mapping to first filter out empty sections (e.g., .filter(({ claims: sectionClaims }) => sectionClaims.length > 0) before .map) or apply a conditional render around the Card to skip cards where sectionClaims.length === 0 (while preserving behavior for any sections you want always visible like "Active Claims"); reference getClaimsBySection, the sectionClaims variable and section.title to locate and change the code.hooks/use-reputation.ts (1)
26-36:offsetis not exposed — pagination beyond the first page is unsupported.
useCompletionHistoryonly forwardslimit, so the backend always receivesoffset = 0. If the history grows beyondDEFAULT_COMPLETION_HISTORY_LIMIT, additional records are silently unreachable from the UI.♻️ Suggested extension for future pagination support
export const useCompletionHistory = ( userId: string, limit = DEFAULT_COMPLETION_HISTORY_LIMIT, + offset = 0, ) => { return useQuery({ - queryKey: REPUTATION_KEYS.completionHistory(userId, limit), - queryFn: () => reputationApi.fetchCompletionHistory(userId, { limit }), + queryKey: REPUTATION_KEYS.completionHistory(userId, limit), // extend key with offset if needed + queryFn: () => reputationApi.fetchCompletionHistory(userId, { limit, offset }), enabled: !!userId, staleTime: 1000 * 60 * 5, }); };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/use-reputation.ts` around lines 26 - 36, useCompletionHistory only forwards limit so backend always gets offset=0; add an offset parameter (default 0) to the hook signature and propagate it into the queryKey (REPUTATION_KEYS.completionHistory(userId, limit, offset)) and the queryFn call (reputationApi.fetchCompletionHistory(userId, { limit, offset })), keep enabled as !!userId and preserve staleTime; ensure callers can pass offset to support pagination.app/profile/[userId]/page.tsx (1)
110-123: Duplicate "Profile Not Found" block is unreachable in practice.After
isLoadingis false and noerror, React Query guaranteesdatais either a value orundefinedonly on the very first render tick (before the query fires). This fallback duplicates lines 80–92 exactly. Extract a shared component or remove the duplication.♻️ Suggested refactor
- if (!reputation) { - return ( - <div className="container mx-auto py-16 text-center"> - <AlertCircle className="w-12 h-12 mx-auto text-muted-foreground mb-4" /> - <h1 className="text-2xl font-bold mb-2">Profile Not Found</h1> - <p className="text-muted-foreground mb-6"> - We could not find a reputation profile for this user. - </p> - <Button asChild variant="outline"> - <Link href="/">Return Home</Link> - </Button> - </div> - ); - }Or extract a shared
<NotFoundState />component and reuse it for both paths.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/profile/`[userId]/page.tsx around lines 110 - 123, The "Profile Not Found" JSX repeated under the `if (!reputation)` check is duplicative of the earlier not-found render (after `isLoading`/`error` handling); extract that markup into a small shared component (e.g. NotFoundState) and replace both occurrences with <NotFoundState /> or remove the second unreachable `if (!reputation)` block entirely. Locate the repeated markup around the `reputation` variable and the earlier not-found branch that uses AlertCircle, Button and Link, move that JSX into a function/component (NotFoundState) and reuse it from both places to eliminate duplication.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/bounties/`[id]/submit/route.ts:
- Around line 90-95: The catch block in the submit route silently drops
exceptions; update the catch to capture the error (e.g., catch (error)) and log
it before returning the 500 response so failures are traceable. Specifically,
modify the catch near the NextResponse.json call in route.ts (the submit
handler) to log a clear message and the error (stack) using console.error or the
existing application logger (e.g., console.error("Error in bounty submit:",
error) or processLogger.error(...)), then return the same NextResponse.json({
error: "Internal Server Error" }, { status: 500 }).
- Around line 59-68: The current check-then-act in route.ts using
BountyStore.getSubmissionsByBounty plus a later BountyStore.addSubmission
creates a TOCTOU race; move the uniqueness enforcement into the store layer by
updating BountyStore.addSubmission to perform an atomic conditional insert (or
enforce a unique constraint on (bountyId, contributorId)) and return a clear
error/boolean when a duplicate exists; update the route handler to call
addSubmission directly and map the store's duplicate response to a 409 JSON
error instead of doing a pre-check with getSubmissionsByBounty.
- Around line 70-81: The submission currently trusts walletAddress from
parsed.data; instead extend the User type returned by getCurrentUser() to
include walletAddress, remove walletAddress from submissionFormSchema so the
form no longer validates/accepts it, and when building the Submission (the
object created in the route handler where you call generateId() and assign
bountyId/contributorId/content/explanation) set walletAddress =
currentUser.walletAddress (retrieved from getCurrentUser()) rather than using
parsed.data.walletAddress; update any types/usages that assume User only had
id/name/email to include the new walletAddress field.
In `@app/api/reputation/`[userId]/completion-history/route.ts:
- Line 37: The current mapping uses pointsEarned: reward which conflates
monetary reward with reputation points (variable reward/rewardAmount); change
pointsEarned to a safe placeholder (0) instead of using reward, and add a TODO
comment near the assignment in the completion-history route handler
(pointsEarned) indicating that a proper conversion or points system must be
implemented later (match the existing placeholder approach used for
completionTimeHours). Ensure you only replace the value at the pointsEarned
property and keep the rest of the response intact.
- Around line 48-52: The current parsing of limit uses
`Number(searchParams.get("limit")) || 50` which treats negative numbers as
truthy and then clamps them to 1 via `Math.max(1, ...)`, causing negative values
to become 1 instead of falling back to the default 50; change the logic around
the `limit` variable so you explicitly parse the value from
`searchParams.get("limit")`, validate it (finite and > 0), and if invalid or <=
0 use the default 50, otherwise clamp the positive parsed value to the maximum
100 (optionally integerify with Math.floor). Update the `limit` calculation (the
code that reads `searchParams.get("limit")` and assigns `limit`) to follow this
parse-validate-fallback-clamp flow so negatives fall back to 50.
- Around line 12-39: The generated record id in bountyToCompletionRecord is
unstable because it includes the pagination-dependent index; remove the index
parameter from bountyToCompletionRecord and change the id construction to use
only bounty.id (e.g., id = `completion-${bounty.id}`) so each bounty maps to a
stable unique id; update all call sites that currently pass an index (the
map/loop that calls bountyToCompletionRecord in this route) to stop passing the
index and call the updated function with just the Bounty argument.
In `@app/profile/`[userId]/page.tsx:
- Around line 64-72: The Skeleton loading placeholders in
app/profile/[userId]/page.tsx use a non-existent Tailwind utility "h-100"
causing zero height; update the Skeleton className values (the two occurrences
using "h-100 md:col-span-1" and "h-100 md:col-span-2") to valid Tailwind heights
such as "h-96 md:col-span-1" / "h-96 md:col-span-2" or use an arbitrary value
like "h-[400px] md:col-span-1" / "h-[400px] md:col-span-2" so the loading
skeletons render with the intended height.
- Around line 74-78: Replace the unsafe cast of error in the
useContributorReputation handling with a proper type guard: import ApiError from
lib/api/errors and use ApiError.isApiError(error) to detect ApiError, then set
isNotFound only when that guard is true and error.status === 404; update the
conditional logic in the error branch (where isNotFound is computed and
subsequent 404 vs generic UI is shown) to rely on that type-guarded check
instead of message.includes or casting.
---
Nitpick comments:
In `@app/api/bounties/`[id]/submit/route.ts:
- Around line 46-51: The allowedModels array is currently created per-request;
hoist it to module scope by moving the const allowedModels = [...] declaration
out of the request handler in route.ts (so it becomes a top-level module
constant) and update any references inside the handler (e.g., validation logic
that uses allowedModels) to use this module-level variable; keep the identifier
allowedModels unchanged to minimize code changes.
In `@app/profile/`[userId]/page.tsx:
- Around line 110-123: The "Profile Not Found" JSX repeated under the `if
(!reputation)` check is duplicative of the earlier not-found render (after
`isLoading`/`error` handling); extract that markup into a small shared component
(e.g. NotFoundState) and replace both occurrences with <NotFoundState /> or
remove the second unreachable `if (!reputation)` block entirely. Locate the
repeated markup around the `reputation` variable and the earlier not-found
branch that uses AlertCircle, Button and Link, move that JSX into a
function/component (NotFoundState) and reuse it from both places to eliminate
duplication.
In `@components/reputation/my-claims.tsx`:
- Around line 62-129: The UI currently renders every section returned by
getClaimsBySection(claims) even when sectionClaims is empty; update the mapping
to first filter out empty sections (e.g., .filter(({ claims: sectionClaims }) =>
sectionClaims.length > 0) before .map) or apply a conditional render around the
Card to skip cards where sectionClaims.length === 0 (while preserving behavior
for any sections you want always visible like "Active Claims"); reference
getClaimsBySection, the sectionClaims variable and section.title to locate and
change the code.
In `@hooks/use-reputation.ts`:
- Around line 26-36: useCompletionHistory only forwards limit so backend always
gets offset=0; add an offset parameter (default 0) to the hook signature and
propagate it into the queryKey (REPUTATION_KEYS.completionHistory(userId, limit,
offset)) and the queryFn call (reputationApi.fetchCompletionHistory(userId, {
limit, offset })), keep enabled as !!userId and preserve staleTime; ensure
callers can pass offset to support pagination.
Comment on lines
+59
to
+68
| const existingSubmission = BountyStore.getSubmissionsByBounty( | ||
| bountyId, | ||
| ).find((s) => s.contributorId === contributorId); | ||
|
|
||
| BountyStore.addSubmission(submission); | ||
| if (existingSubmission) { | ||
| return NextResponse.json( | ||
| { error: "Duplicate submission" }, | ||
| { status: 409 }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
TOCTOU race between duplicate check and write allows double submissions.
Under concurrent requests from the same contributor, both can pass the existingSubmission check before either calls BountyStore.addSubmission, resulting in two persisted submissions. The check-then-act on Lines 59–68 and Line 87 is not atomic.
The BountyStore.addSubmission should either perform an atomic conditional insert, or the store layer should enforce uniqueness per (bountyId, contributorId).
💡 Suggested mitigation (store-level guard)
- BountyStore.addSubmission(submission);
+ const added = BountyStore.addSubmissionIfNotDuplicate(submission);
+ if (!added) {
+ return NextResponse.json({ error: "Duplicate submission" }, { status: 409 });
+ }Move the uniqueness check inside the store's addSubmission as an atomic operation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/bounties/`[id]/submit/route.ts around lines 59 - 68, The current
check-then-act in route.ts using BountyStore.getSubmissionsByBounty plus a later
BountyStore.addSubmission creates a TOCTOU race; move the uniqueness enforcement
into the store layer by updating BountyStore.addSubmission to perform an atomic
conditional insert (or enforce a unique constraint on (bountyId, contributorId))
and return a clear error/boolean when a duplicate exists; update the route
handler to call addSubmission directly and map the store's duplicate response to
a 409 JSON error instead of doing a pre-check with getSubmissionsByBounty.
Comment on lines
+70
to
+81
| const { explanation, walletAddress, githubUrl, demoUrl, attachments } = | ||
| parsed.data; | ||
|
|
||
| } catch { | ||
| return NextResponse.json({ error: 'Internal Server Error' }, { status: 500 }); | ||
| } | ||
| const submission: Submission = { | ||
| id: generateId(), | ||
| bountyId, | ||
| contributorId, | ||
| content: explanation, | ||
| explanation, | ||
| walletAddress, | ||
| githubUrl: githubUrl || undefined, | ||
| demoUrl: demoUrl || undefined, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find and read the route file
find . -name "route.ts" -path "*bounties*submit*" 2>/dev/null | head -5Repository: boundlessfi/bounties
Length of output: 103
🏁 Script executed:
# Search for the file mentioned in the review
fd -e ts "route" | grep -E "bounties.*submit" | head -10Repository: boundlessfi/bounties
Length of output: 101
🏁 Script executed:
# Try a broader search
git ls-files | grep -E "(bounties|submit)" | head -20Repository: boundlessfi/bounties
Length of output: 602
🏁 Script executed:
# Search for getCurrentUser implementation
rg "getCurrentUser" --type ts -lRepository: boundlessfi/bounties
Length of output: 486
🏁 Script executed:
# Read the submit route file
cat -n app/api/bounties/[id]/submit/route.tsRepository: boundlessfi/bounties
Length of output: 3418
🏁 Script executed:
# Read the server-auth file to understand getCurrentUser and User type
cat -n lib/server-auth.tsRepository: boundlessfi/bounties
Length of output: 1132
walletAddress must be sourced from the authenticated user, not the client body.
The submission currently accepts walletAddress from the request body (line 70), allowing any caller to redirect the bounty reward to an arbitrary address. Per the PR context, the wallet address should be "prefilled" from the authenticated user's profile.
However, the User interface returned by getCurrentUser() currently only includes id, name, and email—it lacks a walletAddress field. To properly fix this:
- Extend the
Userinterface to includewalletAddress(fetched from the user's stored profile/database) - Remove
walletAddressfrom the form schema validation insubmissionFormSchema - Assign the wallet address from the authenticated user instead of the parsed form data
The current implementation creates a security vulnerability where reward payments can be intercepted by submitting a different wallet address.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/bounties/`[id]/submit/route.ts around lines 70 - 81, The submission
currently trusts walletAddress from parsed.data; instead extend the User type
returned by getCurrentUser() to include walletAddress, remove walletAddress from
submissionFormSchema so the form no longer validates/accepts it, and when
building the Submission (the object created in the route handler where you call
generateId() and assign bountyId/contributorId/content/explanation) set
walletAddress = currentUser.walletAddress (retrieved from getCurrentUser())
rather than using parsed.data.walletAddress; update any types/usages that assume
User only had id/name/email to include the new walletAddress field.
Comment on lines
+90
to
+95
| } catch { | ||
| return NextResponse.json( | ||
| { error: "Internal Server Error" }, | ||
| { status: 500 }, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Catch block silently swallows errors — add logging.
Any unexpected exception disappears with no server-side trace. This makes diagnosing production failures nearly impossible.
🛠️ Proposed fix
- } catch {
+ } catch (error) {
+ console.error("[POST /api/bounties/:id/submit] Unexpected error:", error);
return NextResponse.json(
{ error: "Internal Server Error" },
{ status: 500 },
);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch { | |
| return NextResponse.json( | |
| { error: "Internal Server Error" }, | |
| { status: 500 }, | |
| ); | |
| } | |
| } catch (error) { | |
| console.error("[POST /api/bounties/:id/submit] Unexpected error:", error); | |
| return NextResponse.json( | |
| { error: "Internal Server Error" }, | |
| { status: 500 }, | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/bounties/`[id]/submit/route.ts around lines 90 - 95, The catch block
in the submit route silently drops exceptions; update the catch to capture the
error (e.g., catch (error)) and log it before returning the 500 response so
failures are traceable. Specifically, modify the catch near the
NextResponse.json call in route.ts (the submit handler) to log a clear message
and the error (stack) using console.error or the existing application logger
(e.g., console.error("Error in bounty submit:", error) or
processLogger.error(...)), then return the same NextResponse.json({ error:
"Internal Server Error" }, { status: 500 }).
Comment on lines
+12
to
+39
| function bountyToCompletionRecord( | ||
| bounty: Bounty, | ||
| index: number, | ||
| ): BountyCompletionRecord { | ||
| const difficulty = bounty.difficulty | ||
| ? (DIFFICULTY_MAP[bounty.difficulty] ?? "BEGINNER") | ||
| : "BEGINNER"; | ||
| const reward = bounty.rewardAmount ?? 0; | ||
| const claimedAt = bounty.claimedAt ?? bounty.createdAt; | ||
| const completedAt = bounty.updatedAt; | ||
|
|
||
| return { | ||
| id: `completion-${bounty.id}-${index}`, | ||
| bountyId: bounty.id, | ||
| bountyTitle: bounty.issueTitle, | ||
| projectName: bounty.projectName, | ||
| projectLogoUrl: bounty.projectLogoUrl, | ||
| difficulty, | ||
| rewardAmount: reward, | ||
| rewardCurrency: bounty.rewardCurrency, | ||
| claimedAt, | ||
| completedAt, | ||
| completionTimeHours: 0, | ||
| maintainerRating: null, | ||
| maintainerFeedback: null, | ||
| pointsEarned: reward, | ||
| }; | ||
| } |
There was a problem hiding this comment.
The generated id is pagination-offset-dependent — same bounty gets different IDs on different pages.
id: \completion-${bounty.id}-${index}`whereindex = offset + i. A bounty at result-position 10 fetched at offset=0(i=9 → id...-9) has a different id when fetched at offset=5(i=0 → id...-5). Since each bounty maps to exactly one record, bounty.id` already guarantees uniqueness; the suffix just adds instability.
🐛 Proposed fix
- id: `completion-${bounty.id}-${index}`,
+ id: `completion-${bounty.id}`,You can also remove the unused index parameter from bountyToCompletionRecord:
function bountyToCompletionRecord(
bounty: Bounty,
- index: number,
): BountyCompletionRecord {And update the call site:
- const records: BountyCompletionRecord[] = paginated.map((b, i) =>
- bountyToCompletionRecord(b, offset + i),
- );
+ const records: BountyCompletionRecord[] = paginated.map((b) =>
+ bountyToCompletionRecord(b),
+ );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/reputation/`[userId]/completion-history/route.ts around lines 12 -
39, The generated record id in bountyToCompletionRecord is unstable because it
includes the pagination-dependent index; remove the index parameter from
bountyToCompletionRecord and change the id construction to use only bounty.id
(e.g., id = `completion-${bounty.id}`) so each bounty maps to a stable unique
id; update all call sites that currently pass an index (the map/loop that calls
bountyToCompletionRecord in this route) to stop passing the index and call the
updated function with just the Bounty argument.
| completionTimeHours: 0, | ||
| maintainerRating: null, | ||
| maintainerFeedback: null, | ||
| pointsEarned: reward, |
There was a problem hiding this comment.
pointsEarned silently equates monetary reward to reputation points.
pointsEarned: reward treats rewardAmount (a payment value) as earned reputation points. Unless points are always 1:1 with reward currency in this domain, this will produce incorrect reputation scores once a real points system is introduced. At minimum, use 0 as a safe placeholder (matching completionTimeHours) and document the TODO.
♻️ Suggested fix
- pointsEarned: reward,
+ pointsEarned: 0, // TODO: derive from actual reputation scoring rules📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pointsEarned: reward, | |
| pointsEarned: 0, // TODO: derive from actual reputation scoring rules |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/reputation/`[userId]/completion-history/route.ts at line 37, The
current mapping uses pointsEarned: reward which conflates monetary reward with
reputation points (variable reward/rewardAmount); change pointsEarned to a safe
placeholder (0) instead of using reward, and add a TODO comment near the
assignment in the completion-history route handler (pointsEarned) indicating
that a proper conversion or points system must be implemented later (match the
existing placeholder approach used for completionTimeHours). Ensure you only
replace the value at the pointsEarned property and keep the rest of the response
intact.
Comment on lines
+48
to
+52
| const limit = Math.min( | ||
| Math.max(1, Number(searchParams.get("limit")) || 50), | ||
| 100, | ||
| ); | ||
| const offset = Math.max(0, Number(searchParams.get("offset")) || 0); |
There was a problem hiding this comment.
Negative limit values are clamped to 1 instead of falling back to the default 50.
Number("-5") || 50 evaluates to -5 (truthy), so Math.max(1, -5) = 1. In contrast, limit=0 correctly falls back to 50 because Number("0") || 50 = 50. The inconsistency means an adversarial or buggy caller passing a negative limit gets the minimum (1) instead of the intended default.
🐛 Proposed fix
- const limit = Math.min(
- Math.max(1, Number(searchParams.get("limit")) || 50),
- 100,
- );
- const offset = Math.max(0, Number(searchParams.get("offset")) || 0);
+ const rawLimit = Number(searchParams.get("limit"));
+ const limit = Math.min(Math.max(1, rawLimit > 0 ? rawLimit : 50), 100);
+ const rawOffset = Number(searchParams.get("offset"));
+ const offset = Math.max(0, rawOffset > 0 ? rawOffset : 0);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const limit = Math.min( | |
| Math.max(1, Number(searchParams.get("limit")) || 50), | |
| 100, | |
| ); | |
| const offset = Math.max(0, Number(searchParams.get("offset")) || 0); | |
| const rawLimit = Number(searchParams.get("limit")); | |
| const limit = Math.min(Math.max(1, rawLimit > 0 ? rawLimit : 50), 100); | |
| const rawOffset = Number(searchParams.get("offset")); | |
| const offset = Math.max(0, rawOffset > 0 ? rawOffset : 0); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/reputation/`[userId]/completion-history/route.ts around lines 48 -
52, The current parsing of limit uses `Number(searchParams.get("limit")) || 50`
which treats negative numbers as truthy and then clamps them to 1 via
`Math.max(1, ...)`, causing negative values to become 1 instead of falling back
to the default 50; change the logic around the `limit` variable so you
explicitly parse the value from `searchParams.get("limit")`, validate it (finite
and > 0), and if invalid or <= 0 use the default 50, otherwise clamp the
positive parsed value to the maximum 100 (optionally integerify with
Math.floor). Update the `limit` calculation (the code that reads
`searchParams.get("limit")` and assigns `limit`) to follow this
parse-validate-fallback-clamp flow so negatives fall back to 50.
Comment on lines
+64
to
+72
| <div className="container mx-auto py-8"> | ||
| <Skeleton className="h-10 w-32 mb-8" /> | ||
| <div className="grid grid-cols-1 md:grid-cols-3 gap-8"> | ||
| <Skeleton className="h-100 md:col-span-1" /> | ||
| <Skeleton className="h-100 md:col-span-2" /> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
h-100 is not a standard Tailwind utility — loading skeleton renders invisible.
Tailwind's height scale tops out at h-96 (24rem). h-100 produces no CSS without a custom config entry, so both skeletons have zero height during loading.
🐛 Proposed fix
- <Skeleton className="h-100 md:col-span-1" />
- <Skeleton className="h-100 md:col-span-2" />
+ <Skeleton className="h-96 md:col-span-1" />
+ <Skeleton className="h-96 md:col-span-2" />Or use an arbitrary value such as h-[400px] to match the actual card height.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div className="container mx-auto py-8"> | |
| <Skeleton className="h-10 w-32 mb-8" /> | |
| <div className="grid grid-cols-1 md:grid-cols-3 gap-8"> | |
| <Skeleton className="h-100 md:col-span-1" /> | |
| <Skeleton className="h-100 md:col-span-2" /> | |
| </div> | |
| </div> | |
| ); | |
| } | |
| <div className="container mx-auto py-8"> | |
| <Skeleton className="h-10 w-32 mb-8" /> | |
| <div className="grid grid-cols-1 md:grid-cols-3 gap-8"> | |
| <Skeleton className="h-96 md:col-span-1" /> | |
| <Skeleton className="h-96 md:col-span-2" /> | |
| </div> | |
| </div> | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/profile/`[userId]/page.tsx around lines 64 - 72, The Skeleton loading
placeholders in app/profile/[userId]/page.tsx use a non-existent Tailwind
utility "h-100" causing zero height; update the Skeleton className values (the
two occurrences using "h-100 md:col-span-1" and "h-100 md:col-span-2") to valid
Tailwind heights such as "h-96 md:col-span-1" / "h-96 md:col-span-2" or use an
arbitrary value like "h-[400px] md:col-span-1" / "h-[400px] md:col-span-2" so
the loading skeletons render with the intended height.
Comment on lines
+74
to
+78
| if (error) { | ||
| // Check if it's a 404 (Not Found) | ||
| const apiError = error as { status?: number; message?: string }; | ||
| const isNotFound = | ||
| apiError?.status === 404 || apiError?.message?.includes("404"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, explore the repository structure to understand the layout
find . -type f -name "*.ts" -o -name "*.tsx" | grep -E "(api|client)" | head -20Repository: boundlessfi/bounties
Length of output: 878
🏁 Script executed:
# Look for the lib directory and API client code
find . -type d -name "lib" | head -10
ls -la lib/ 2>/dev/null || echo "lib not found at root"Repository: boundlessfi/bounties
Length of output: 1950
🏁 Script executed:
# List files in lib/api directory
ls -la lib/api/Repository: boundlessfi/bounties
Length of output: 662
🏁 Script executed:
# Examine the API client code
cat -n lib/api/client.tsRepository: boundlessfi/bounties
Length of output: 4964
🏁 Script executed:
# Examine the error classes
cat -n lib/api/errors.tsRepository: boundlessfi/bounties
Length of output: 2369
🏁 Script executed:
# Examine the profile page to see the error handling context
cat -n app/profile/[userId]/page.tsx | sed -n '1,100p'Repository: boundlessfi/bounties
Length of output: 4140
🏁 Script executed:
# Find the use-reputation hook
find . -name "use-reputation*" -type fRepository: boundlessfi/bounties
Length of output: 89
🏁 Script executed:
# Examine the use-reputation hook
cat -n hooks/use-reputation.tsRepository: boundlessfi/bounties
Length of output: 3436
🏁 Script executed:
# Examine the reputation API
cat -n lib/api/reputation.tsRepository: boundlessfi/bounties
Length of output: 2253
Use proper error type guards instead of unsafe casting.
The error from useContributorReputation can be either ApiError (with a status field) or NetworkError (without status). Casting to { status?: number; message?: string } masks this distinction. Additionally, the fallback check message?.includes("404") won't match ApiError's default message "Resource not found".
Instead, import ApiError from lib/api/errors and check the error type first:
if (error) {
const isNotFound = ApiError.isApiError(error) && error.status === 404;
if (isNotFound) {
// Show 404-specific UI
} else {
// Show generic error UI
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/profile/`[userId]/page.tsx around lines 74 - 78, Replace the unsafe cast
of error in the useContributorReputation handling with a proper type guard:
import ApiError from lib/api/errors and use ApiError.isApiError(error) to detect
ApiError, then set isNotFound only when that guard is true and error.status ===
404; update the conditional logic in the error branch (where isNotFound is
computed and subsequent 404 vs generic UI is shown) to rely on that type-guarded
check instead of message.includes or casting.
This was referenced Feb 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #81
Summary by CodeRabbit
New Features
Improvements