Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughExtracts route DB logic into server-side remotes (bills, dashboard, payments, images, common), adds ULID/dueDate validators and tests, upgrades SvelteKit/tooling and runtime deps, enables experimental async/remoteFunctions, refactors client components to consume remotes and adjusts form/upload flows. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Remote as Server Remote (uploadImage)
participant Storage as Supabase Storage
participant DB as Database
Client->>Remote: submit uploadImage (paymentId, householdId, proofFile, amount, notes)
Remote->>Storage: create signed upload URL / upload file to PAYMENT_BUCKET_NAME (householdId/paymentId)
Storage-->>Remote: upload response (success or error)
alt upload success
Remote->>DB: getImageIdByPath(path) -> SELECT id FROM objects WHERE bucketId=... AND name=path
DB-->>Remote: imageId
Remote->>DB: UPDATE payments SET proofImage=imageId, paidAt=now(), amount=..., notes=... WHERE paymentId AND household scoped to user
DB-->>Remote: updated payment row
Remote-->>Client: return updated payment
else upload error
Remote-->>Client: error (500)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
jhechtf
force-pushed
the
feat/begin-move-to-remote-functions
branch
from
a1d4698 to
f4d4c90
Compare
jhechtf
force-pushed
the
feat/begin-move-to-remote-functions
branch
from
f4d4c90 to
71435de
Compare
This was referenced Apr 4, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 14
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/website/src/routes/dashboard/payments/+page.server.ts (2)
197-209:⚠️ Potential issue | 🔴 CriticalTransaction is not awaited, causing fire-and-forget execution.
The
db.transaction()call is not awaited, meaning the response returns immediately (return {}) while the transaction runs in the background. If the transaction fails or rolls back, the client won't know, and errors won't propagate properly.🐛 Proposed fix to await the transaction
- db.transaction(async (tx) => { + await db.transaction(async (tx) => { try { const res = await makeMultiplePayments( tx, args, locals.supabase, session, ); console.info(res); } catch { tx.rollback(); } }); return {};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/payments/`+page.server.ts around lines 197 - 209, The db.transaction call is being invoked without awaiting it, which causes fire-and-forget behavior; update the caller in +page.server.ts to await the promise returned by db.transaction so the server waits for makeMultiplePayments to complete and any thrown errors or rollbacks from tx.rollback propagate; ensure you also rethrow or return the transaction result from the async callback (where makeMultiplePayments is called) so callers receive success/failure information rather than returning immediately.
38-38:⚠️ Potential issue | 🟡 MinorRemove profanity from error logging.
The error message contains unprofessional language. Consider using a more descriptive message.
✏️ Suggested fix
- console.error('SHITS FUCKED'); + console.error('Form validation failed');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/payments/`+page.server.ts at line 38, Replace the unprofessional console.error call ("SHITS FUCKED") in +page.server.ts with a descriptive, professional error log that gives context and includes the actual error object (e.g., use console.error or your app logger to log "Failed to [action]" plus the error variable), and ensure this happens in the same catch/error handling block where the current console.error is located so callers can diagnose the failure.apps/website/src/routes/dashboard/+page.svelte (1)
306-306:⚠️ Potential issue | 🟠 MajorPotential null reference on
bill.payment.id.Line 306 accesses
bill.payment.idunconditionally, butbill.paymentcan benullbased on the filter logic at line 81. This will throw a runtime error for bills without payments.🐛 Proposed fix with null check
- href={`/dashboard/payments/create/${bill.payment.id}`} + href={`/dashboard/payments/create/${bill.payment?.id}`}Additionally, consider hiding or disabling the "Pay" link entirely when there's no payment record.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/`+page.svelte at line 306, The href uses bill.payment.id unguarded which can be null; update the rendering for the Pay link so it first checks bill.payment (e.g., guard on bill.payment !== null/undefined) before accessing bill.payment.id and either omit/disable the anchor or render a non-clickable element when there's no payment; ensure the conditional is applied where the href is created (the template that references bill.payment.id) and also hide or disable the "Pay" link UI when bill.payment is null to prevent runtime errors and bad UX.
🧹 Nitpick comments (5)
apps/website/src/routes/dashboard/payments/+page.svelte (1)
73-78: Consider adding error handling for the async boundary.The
<svelte:boundary>has apending()snippet for loading state, but no error handling. IfgetCurrentPayments()throws, the error will propagate unhandled. Consider adding afailedsnippet or wrapping with try/catch.♻️ Suggested addition for error handling
<svelte:boundary> {`#snippet` pending()} {`#each` Array(5)} <div class="card animate-pulse h-16"> </div> {/each} {/snippet} + {`#snippet` failed(error)} + <div class="card variant-filled-error p-4"> + <p>Failed to load payments. Please try again.</p> + </div> + {/snippet} {`#each` await getCurrentPayments() as payment}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/payments/`+page.svelte around lines 73 - 78, The async boundary currently only provides a pending() snippet for loading but no failure handling, so add a failed snippet to the <svelte:boundary> that takes the error (e) and renders a user-friendly error state (and optional retry) when getCurrentPayments() throws; locate the <svelte:boundary> block in +page.svelte and add a {`#snippet` failed(e)} ... {/snippet} (or wrap the getCurrentPayments call in a try/catch in the load/action that returns an error state) so errors are caught and displayed instead of bubbling unhandled.apps/website/src/lib/remotes/bills.remote.ts (2)
38-38: Empty type alias serves no purpose.
billValidatoris defined as an empty object type{}and appears unused. Either implement it properly or remove it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/bills.remote.ts` at line 38, The exported type alias billValidator is an empty object type and unused; either remove the unused export to avoid dead code or replace it with a meaningful shape and use it where intended—locate the billValidator symbol in this module, delete the export if there are no references, or redefine it as a proper interface/type that matches the expected validator structure (e.g., fields and method signatures) and update any functions that should accept or return it to use the new type.
40-44: IncompletecreateOrUpdateBillimplementation.The command handler is empty. This appears to be placeholder code.
Would you like me to help implement the
createOrUpdateBillcommand logic, or should this be tracked as a follow-up task?🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/bills.remote.ts` around lines 40 - 44, The createOrUpdateBill command handler is empty; implement logic inside createOrUpdateBill to accept the optional 'id' (from the type schema using ulid) and either create a new bill when id is absent or update the existing bill when id is present: validate incoming payload, generate a new ULID if creating, call your data-layer functions (e.g., bills.createBill or bills.updateBill — locate your repository/DB methods used elsewhere for consistency), handle not-found on update with a clear error, and return the created/updated bill object; ensure errors are caught and rethrown or logged appropriately and the command uses the same command(...) calling pattern as other remote handlers.apps/website/src/lib/remotes/common.remote.ts (1)
1-5: Remove unused imports.
formandtypeare imported but not used in this file.✏️ Suggested fix
-import { query, getRequestEvent, form } from '$app/server'; +import { query, getRequestEvent } from '$app/server'; import schema from '@sungmanito/db'; import { db } from '../server/db'; import { eq, getTableColumns } from 'drizzle-orm'; -import { type } from 'arktype';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/common.remote.ts` around lines 1 - 5, Remove the unused imports `form` and `type`: edit the import lists so you no longer import `form` from '$app/server' and `type` from 'arktype', leaving the other imports (`query`, `getRequestEvent`, `schema`, `db`, `eq`, `getTableColumns`) intact; this will clean up unused symbols and avoid linter warnings in the module containing the functions that reference `query`, `getRequestEvent`, `schema`, `db`, `eq`, and `getTableColumns`.apps/website/src/lib/remotes/payments.remote.ts (1)
214-218: Remove redundanttx.rollback()and rely on automatic rollback.The pattern
tx.rollback(); error(...)is redundant. SvelteKit'serror()throws immediately, halting execution, and Drizzle automatically rolls back transactions on any thrown error. The explicit rollback call is unnecessary and makes intent less clear. Simply callingerror()is sufficient and clearer.♻️ Simplified control flow
if (res.error) { console.error('Failed to remove image from storage:', res.error); - tx.rollback(); error(500, 'Failed to remove image from storage'); }The same pattern appears at line 239–240; apply the same simplification there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 214 - 218, Remove the explicit tx.rollback() calls where the code checks res.error (the branches that call error(500, 'Failed to remove image from storage') when res.error is truthy); rely on SvelteKit's error() to throw and Drizzle's automatic transaction rollback instead. Locate the two occurrences referencing res.error and tx.rollback() (one around the block containing console.error('Failed to remove image from storage:', res.error) and error(500, ...), and the second occurrence at the later check), delete only the tx.rollback() calls and keep the console.error(...) and error(500, 'Failed to remove image from storage') calls unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/website/package.json`:
- Line 80: Remove the unused dependencies "ulidx" and "valibot" from the
apps/website package.json by deleting their entries from the
dependencies/devDependencies section, run your package manager to update the
lockfile (npm install / yarn install), and confirm there are no imports/usages
of ulidx or valibot by searching the codebase (if any usages exist, convert or
remove them before uninstalling). Ensure package.json still contains "arktype":
"2.1.23" as-is and commit the updated package.json and lockfile.
- Around line 38-39: modalify.svelte still uses the old v5 createQuery signature
(passing the config object directly) which breaks under `@tanstack/svelte-query`
v6; update the createQuery call in modalify.svelte (the variable named query
which is wrapped by $derived) to pass a function that returns the config object
(i.e., createQuery(() => ({ queryKey: ['modalify', url], queryFn: async () => {
const response = await preloadData(url); if (response.type === 'loaded' &&
response.status === 200) return response.data as Data; }, staleTime: 5000,
enabled: open }))). Ensure you reference the same symbols (createQuery,
$derived, preloadData, url, open, Data) so the change is applied to the correct
spot.
In `@apps/website/src/lib/remotes/bills.remote.ts`:
- Line 7: The import and usage of ulid (from 'ulidx') is wrong because ulid()
generates IDs rather than validating them; replace the ulid import with the
ulidValidator used elsewhere (import { ulidValidator } from
'$lib/typesValidators') and update any arktype schemas in bills.remote.ts that
currently reference ulid (including the schema usage around the block referenced
at lines ~40-44) to use ulidValidator instead; also remove the unused ulid
import so the module only imports the validator.
In `@apps/website/src/lib/remotes/common.remote.ts`:
- Around line 16-18: The current authentication check throws a plain Error (the
if block checking session / session.user and the throw new Error('User not
authenticated')) which doesn't produce a proper HTTP 401 response; import error
from '@sveltejs/kit' and replace the throw new Error call with error(401, 'User
not authenticated') (ensure the import for error is added at top and the check
remains on session / session.user so unauthorized requests return the proper
HTTP 401).
In `@apps/website/src/lib/remotes/dashboard.remote.ts`:
- Around line 15-17: The status expression incorrectly compares only
day-of-month (schema.bills.dueDate) to today.getDate(); change the logic in the
status SQL expression to construct a full date for the current billing period
and compare that to the current date and to whether there is a payment row for
the current month. Specifically: keep the paid check on schema.payments.paidAt
first; then determine if a payment record exists for the current month (the same
filter used in the payments join) and if so mark 'overdue' only when
current_date > make_date(current_year, current_month, schema.bills.dueDate); if
no payment exists for the current month also mark 'overdue' when current_date >
that constructed due date; otherwise mark 'upcoming'. Use unique symbols
schema.payments.paidAt, schema.bills.dueDate, today.getDate()/current_date and
the payments join/month filter to locate where to update the SQL case
expression.
- Around line 34-38: The month filter in the payments join only compares the
month number (inside the and(...) call) and overlooks the year, so add a second
condition comparing extract('year' from ${schema.payments.forMonthD}) to
today.getFullYear() (same place where today.getMonth()+1 is used) so the
and(...) includes both month and year checks; update the and(...) expression
that contains sql`extract('month' from ${schema.payments.forMonthD})` to also
include eq(sql`extract('year' from ${schema.payments.forMonthD})`,
today.getFullYear()) to prevent matching previous years.
In `@apps/website/src/lib/remotes/images.remote.ts`:
- Around line 25-34: The function removeImageByPath currently validates an ulid
(ulidValidator) but never uses it — event.locals.supabase.storage.remove is
called with an empty array — so nothing is deleted; fix by passing the actual
file path(s) to storage.remove (e.g., remove([id]) or map the validated ULID to
the correct storage path) and update the parameter name and JSDoc to match
whether the function expects a ULID or a path; ensure getRequestEvent and
PAYMENT_BUCKET_NAME usage remains the same and keep error handling for res.error
unchanged.
In `@apps/website/src/lib/remotes/payments.remote.ts`:
- Line 15: Remove the artificial 1.5s delay implemented by awaiting new
Promise((r) => setTimeout(r, 1500)) in payments.remote.ts: either delete that
await line entirely or gate it behind a development-only check (e.g., only run
if NODE_ENV !== 'production') and add a TODO comment explaining its demo purpose
so it won't be shipped to production.
- Around line 34-37: The month-only filter in the payments query (using
eq(sql`extract('month' from ${schema.payments.forMonthD})`, new
Date().getMonth() + 1)) will match any year; update the predicate to also
constrain the year by adding a companion check using extract('year' from
${schema.payments.forMonthD}) === new Date().getFullYear() (combine both with
AND where the eq(...) for month and a new eq(...) for year are applied) so
payments are filtered by both month and year in the same query.
- Line 67: The pipe on the amount field incorrectly uses (s) => s === '' && 0
which yields false for non-empty inputs; update the transform for the amount
property so it returns 0 when s is the empty string and otherwise returns the
original value (e.g., use a conditional expression) — locate the amount
definition in payments.remote.ts and replace the current pipe transformer for
amount with one that returns s === '' ? 0 : s.
- Around line 28-33: The query building uses
inArray(schema.payments.householdId, userHouseholds.map((h) => h.id)) which will
produce an invalid/empty IN () when userHouseholds is empty; modify the code in
payments.remote.ts to guard before constructing the .where(...) clause (or
before invoking the function that builds this query): if userHouseholds.length
=== 0 return an empty result (or short-circuit the query) instead of calling
inArray, otherwise proceed as before; apply the same defensive check wherever
inArray(schema.payments.householdId, userHouseholds.map(...)) or similar inArray
calls are used.
In `@apps/website/src/routes/dashboard/`+page.server.ts:
- Line 6: Remove the unused imports getTableColumns and asc from the drizzle-orm
import list in the module (the import that currently includes sql, and, eq,
getTableColumns, asc); edit the import statement so it only imports sql, and,
and eq to eliminate the unused symbols getTableColumns and asc.
In `@apps/website/src/routes/dashboard/`+page.svelte:
- Around line 70-77: The queryKey used in the createQuery call for
billsWithStatus is inappropriate and should be renamed to a descriptive,
professional key; update the queryKey array in the createQuery invocation (where
billsWithStatus is defined) from ['fuckers'] to a meaningful identifier such as
['household-bills'] or ['dashboard-bills'] so the key reflects the data being
fetched and avoids offensive language.
- Line 41: Remove the development debug call $inspect(data.groupings) from the
Svelte page; locate the expression that references $inspect (the standalone
statement using data.groupings) and delete it so no debug-only inspector runs in
production, and verify there are no remaining unused imports or references to
$inspect in the +page.svelte component (e.g., any top-level $inspect(...)
statements).
---
Outside diff comments:
In `@apps/website/src/routes/dashboard/`+page.svelte:
- Line 306: The href uses bill.payment.id unguarded which can be null; update
the rendering for the Pay link so it first checks bill.payment (e.g., guard on
bill.payment !== null/undefined) before accessing bill.payment.id and either
omit/disable the anchor or render a non-clickable element when there's no
payment; ensure the conditional is applied where the href is created (the
template that references bill.payment.id) and also hide or disable the "Pay"
link UI when bill.payment is null to prevent runtime errors and bad UX.
In `@apps/website/src/routes/dashboard/payments/`+page.server.ts:
- Around line 197-209: The db.transaction call is being invoked without awaiting
it, which causes fire-and-forget behavior; update the caller in +page.server.ts
to await the promise returned by db.transaction so the server waits for
makeMultiplePayments to complete and any thrown errors or rollbacks from
tx.rollback propagate; ensure you also rethrow or return the transaction result
from the async callback (where makeMultiplePayments is called) so callers
receive success/failure information rather than returning immediately.
- Line 38: Replace the unprofessional console.error call ("SHITS FUCKED") in
+page.server.ts with a descriptive, professional error log that gives context
and includes the actual error object (e.g., use console.error or your app logger
to log "Failed to [action]" plus the error variable), and ensure this happens in
the same catch/error handling block where the current console.error is located
so callers can diagnose the failure.
---
Nitpick comments:
In `@apps/website/src/lib/remotes/bills.remote.ts`:
- Line 38: The exported type alias billValidator is an empty object type and
unused; either remove the unused export to avoid dead code or replace it with a
meaningful shape and use it where intended—locate the billValidator symbol in
this module, delete the export if there are no references, or redefine it as a
proper interface/type that matches the expected validator structure (e.g.,
fields and method signatures) and update any functions that should accept or
return it to use the new type.
- Around line 40-44: The createOrUpdateBill command handler is empty; implement
logic inside createOrUpdateBill to accept the optional 'id' (from the type
schema using ulid) and either create a new bill when id is absent or update the
existing bill when id is present: validate incoming payload, generate a new ULID
if creating, call your data-layer functions (e.g., bills.createBill or
bills.updateBill — locate your repository/DB methods used elsewhere for
consistency), handle not-found on update with a clear error, and return the
created/updated bill object; ensure errors are caught and rethrown or logged
appropriately and the command uses the same command(...) calling pattern as
other remote handlers.
In `@apps/website/src/lib/remotes/common.remote.ts`:
- Around line 1-5: Remove the unused imports `form` and `type`: edit the import
lists so you no longer import `form` from '$app/server' and `type` from
'arktype', leaving the other imports (`query`, `getRequestEvent`, `schema`,
`db`, `eq`, `getTableColumns`) intact; this will clean up unused symbols and
avoid linter warnings in the module containing the functions that reference
`query`, `getRequestEvent`, `schema`, `db`, `eq`, and `getTableColumns`.
In `@apps/website/src/lib/remotes/payments.remote.ts`:
- Around line 214-218: Remove the explicit tx.rollback() calls where the code
checks res.error (the branches that call error(500, 'Failed to remove image from
storage') when res.error is truthy); rely on SvelteKit's error() to throw and
Drizzle's automatic transaction rollback instead. Locate the two occurrences
referencing res.error and tx.rollback() (one around the block containing
console.error('Failed to remove image from storage:', res.error) and error(500,
...), and the second occurrence at the later check), delete only the
tx.rollback() calls and keep the console.error(...) and error(500, 'Failed to
remove image from storage') calls unchanged.
In `@apps/website/src/routes/dashboard/payments/`+page.svelte:
- Around line 73-78: The async boundary currently only provides a pending()
snippet for loading but no failure handling, so add a failed snippet to the
<svelte:boundary> that takes the error (e) and renders a user-friendly error
state (and optional retry) when getCurrentPayments() throws; locate the
<svelte:boundary> block in +page.svelte and add a {`#snippet` failed(e)} ...
{/snippet} (or wrap the getCurrentPayments call in a try/catch in the
load/action that returns an error state) so errors are caught and displayed
instead of bubbling unhandled.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f380fe0a-cd34-4ca9-9ec9-1352fb4aaf62
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
apps/website/package.jsonapps/website/src/lib/components/drawerify/drawerify.svelteapps/website/src/lib/components/monthDropdown/monthDropdown.svelteapps/website/src/lib/remotes/bills.remote.tsapps/website/src/lib/remotes/common.remote.tsapps/website/src/lib/remotes/dashboard.remote.tsapps/website/src/lib/remotes/images.remote.tsapps/website/src/lib/remotes/payments.remote.tsapps/website/src/lib/types.test.tsapps/website/src/lib/typesValidators.tsapps/website/src/lib/util/ark-utils.tsapps/website/src/routes/dashboard/+page.server.tsapps/website/src/routes/dashboard/+page.svelteapps/website/src/routes/dashboard/bills/+page.svelteapps/website/src/routes/dashboard/payments/+page.server.tsapps/website/src/routes/dashboard/payments/+page.svelteapps/website/src/routes/dashboard/payments/create/[id=ulid]/+page.svelteapps/website/svelte.config.jspackage.json
💤 Files with no reviewable changes (1)
- apps/website/src/lib/util/ark-utils.ts
Comment on lines
+38
to
+39
| "@sveltejs/kit": "^2.47.0", | ||
| "@sveltejs/vite-plugin-svelte": "^6.2.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for any remaining v5-style createQuery usages (object argument instead of function)
rg -n 'createQuery\s*\(\s*\{' --type=svelte --type=tsRepository: sungmanito/mono
Length of output: 89
🏁 Script executed:
#!/bin/bash
# Find modalify.svelte and related files
fd -i modalifyRepository: sungmanito/mono
Length of output: 157
🏁 Script executed:
#!/bin/bash
# Search for createQuery usage in all files (don't restrict by type)
rg -n 'createQuery\s*\(' --max-count=20Repository: sungmanito/mono
Length of output: 330
🏁 Script executed:
#!/bin/bash
# Check the actual package.json to see current dependency versions
cat apps/website/package.json | grep -E '(`@sveltejs/kit`|@sveltejs/vite-plugin-svelte|vite|@tanstack/svelte-query|arktype)'Repository: sungmanito/mono
Length of output: 437
🏁 Script executed:
#!/bin/bash
cat -n apps/website/src/lib/components/modalify/modalify.svelte | head -60Repository: sungmanito/mono
Length of output: 1989
🏁 Script executed:
#!/bin/bash
# Check what's on lines 66 and 78 of package.json
sed -n '60,85p' apps/website/package.json | cat -nRepository: sungmanito/mono
Length of output: 1006
Fix createQuery usage in modalify.svelte for v6 API compatibility.
The @tanstack/svelte-query v5→v6 migration requires wrapping the configuration object in a function. drawerify.svelte and dashboard/+page.svelte have been correctly updated to use createQuery(() => ({...})), but modalify.svelte (line 41) still uses the v5 pattern createQuery({...}) and will break silently in v6. Update it to:
const query = $derived(
createQuery(() => ({
queryKey: ['modalify', url],
queryFn: async () => {
const response = await preloadData(url);
if (response.type === 'loaded' && response.status === 200) {
return response.data as Data;
}
},
staleTime: 5000,
enabled: open,
})),
);This is critical because the new @tanstack/svelte-query v6.0.0 dependency in package.json requires this pattern.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/package.json` around lines 38 - 39, modalify.svelte still uses
the old v5 createQuery signature (passing the config object directly) which
breaks under `@tanstack/svelte-query` v6; update the createQuery call in
modalify.svelte (the variable named query which is wrapped by $derived) to pass
a function that returns the config object (i.e., createQuery(() => ({ queryKey:
['modalify', url], queryFn: async () => { const response = await
preloadData(url); if (response.type === 'loaded' && response.status === 200)
return response.data as Data; }, staleTime: 5000, enabled: open }))). Ensure you
reference the same symbols (createQuery, $derived, preloadData, url, open, Data)
so the change is applied to the correct spot.
| "@tanstack/svelte-query": "^6.0.0", | ||
| "@vercel/edge-config": "^0.4.1", | ||
| "arktype": "2.0.4", | ||
| "arktype": "2.1.23", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if ulidx is actually imported/used
rg -n "from ['\"]ulidx['\"]" --type=ts --type=svelte
# Check if valibot is actually imported/used
rg -n "from ['\"]valibot['\"]" --type=ts --type=svelteRepository: sungmanito/mono
Length of output: 120
🏁 Script executed:
#!/bin/bash
# Search for ulidx imports - check all relevant files
echo "=== Searching for ulidx imports ==="
rg "ulidx" --type=ts --type=tsx --type=js --type=jsx -n
echo ""
echo "=== Searching for valibot imports ==="
rg "valibot" --type=ts --type=tsx --type=js --type=jsx -n
echo ""
echo "=== Checking package.json for these dependencies ==="
rg -A 5 '"ulidx"|"valibot"' apps/website/package.jsonRepository: sungmanito/mono
Length of output: 313
🏁 Script executed:
#!/bin/bash
echo "=== Searching for arktype usage ==="
rg "arktype" --type=ts --type=js -n
echo ""
echo "=== Checking arktype in package.json ==="
rg "arktype" apps/website/package.json
echo ""
echo "=== Check git history for arktype version change ==="
cd apps/website && git log -1 --oneline -- package.json || echo "Git history not available in sandbox"Repository: sungmanito/mono
Length of output: 3507
Remove unused dependencies ulidx and valibot from package.json.
These dependencies were added but are not imported anywhere in the codebase. They should be removed to reduce bundle size and maintenance overhead. The arktype upgrade to 2.1.23 is appropriate—it's actively used throughout the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/package.json` at line 80, Remove the unused dependencies "ulidx"
and "valibot" from the apps/website package.json by deleting their entries from
the dependencies/devDependencies section, run your package manager to update the
lockfile (npm install / yarn install), and confirm there are no imports/usages
of ulidx or valibot by searching the codebase (if any usages exist, convert or
remove them before uninstalling). Ensure package.json still contains "arktype":
"2.1.23" as-is and commit the updated package.json and lockfile.
| import { and, eq, getTableColumns } from 'drizzle-orm'; | ||
| import { getUser } from './common.remote'; | ||
| import { type } from 'arktype'; | ||
| import { ulid } from 'ulidx'; |
There was a problem hiding this comment.
Incorrect use of ulid as a validator.
ulid from ulidx is a function that generates ULIDs, not a validator. Using it in an arktype schema will not validate input correctly. Use ulidValidator from $lib/typesValidators as done in other remotes (e.g., payments.remote.ts).
🐛 Proposed fix
-import { ulid } from 'ulidx';
+import { ulidValidator } from '$lib/typesValidators';
// ... rest of file ...
export const createOrUpdateBill = command(
type({
- 'id?': ulid,
+ 'id?': ulidValidator,
}),
async () => {},
);Also applies to: 40-44
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/lib/remotes/bills.remote.ts` at line 7, The import and usage
of ulid (from 'ulidx') is wrong because ulid() generates IDs rather than
validating them; replace the ulid import with the ulidValidator used elsewhere
(import { ulidValidator } from '$lib/typesValidators') and update any arktype
schemas in bills.remote.ts that currently reference ulid (including the schema
usage around the block referenced at lines ~40-44) to use ulidValidator instead;
also remove the unused ulid import so the module only imports the validator.
Comment on lines
+16
to
+18
| if (!session || !session.user) { | ||
| throw new Error('User not authenticated'); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Use SvelteKit's error() for proper HTTP error responses.
Throwing a plain Error won't produce a proper HTTP 401 response. Use error() from @sveltejs/kit for consistent error handling across the application.
♻️ Proposed fix
+import { error } from '@sveltejs/kit';
+
export const getUser = query(async () => {
const event = getRequestEvent();
const session = await event.locals.getSession();
if (!session || !session.user) {
- throw new Error('User not authenticated');
+ throw error(401, 'User not authenticated');
}📝 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
| if (!session || !session.user) { | |
| throw new Error('User not authenticated'); | |
| } | |
| import { error } from '@sveltejs/kit'; | |
| export const getUser = query(async () => { | |
| const event = getRequestEvent(); | |
| const session = await event.locals.getSession(); | |
| if (!session || !session.user) { | |
| throw error(401, 'User not authenticated'); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/lib/remotes/common.remote.ts` around lines 16 - 18, The
current authentication check throws a plain Error (the if block checking session
/ session.user and the throw new Error('User not authenticated')) which doesn't
produce a proper HTTP 401 response; import error from '@sveltejs/kit' and
replace the throw new Error call with error(401, 'User not authenticated')
(ensure the import for error is added at top and the check remains on session /
session.user so unauthorized requests return the proper HTTP 401).
Comment on lines
+15
to
+17
| status: sql< | ||
| 'overdue' | 'upcoming' | 'paid' | ||
| >`case when ${schema.payments.paidAt} is not null then 'paid'::text when ${schema.bills.dueDate} < ${today.getDate()} then 'overdue'::text else 'upcoming'::text end`, |
There was a problem hiding this comment.
Flawed overdue status calculation - compares only day-of-month.
The status logic compares bills.dueDate (an integer 1-31 representing day-of-month) against today.getDate(). This doesn't account for month boundaries correctly.
Example: On February 5th, a bill with dueDate = 30 would be marked as 'upcoming' (30 > 5), even if the payment for January is past due.
The payment join filters to current month only, but the overdue logic needs to consider whether the current month's payment exists and whether we're past the due date for the current month.
💡 Suggested approach
The status should consider:
- If payment exists and
paidAtis not null → 'paid' - If payment exists for current month and today > dueDate → 'overdue'
- If no payment for current month and we're past the due date → 'overdue'
- Otherwise → 'upcoming'
status: sql<
'overdue' | 'upcoming' | 'paid'
- >`case when ${schema.payments.paidAt} is not null then 'paid'::text when ${schema.bills.dueDate} < ${today.getDate()} then 'overdue'::text else 'upcoming'::text end`,
+ >`case
+ when ${schema.payments.paidAt} is not null then 'paid'::text
+ when ${schema.bills.dueDate} < ${today.getDate()} then 'overdue'::text
+ else 'upcoming'::text
+ end`,Note: This still has the fundamental issue. Consider whether "overdue" should be calculated based on whether a payment record exists for the current month AND today > dueDate, vs bills that have no payment record at all for the current month.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/lib/remotes/dashboard.remote.ts` around lines 15 - 17, The
status expression incorrectly compares only day-of-month (schema.bills.dueDate)
to today.getDate(); change the logic in the status SQL expression to construct a
full date for the current billing period and compare that to the current date
and to whether there is a payment row for the current month. Specifically: keep
the paid check on schema.payments.paidAt first; then determine if a payment
record exists for the current month (the same filter used in the payments join)
and if so mark 'overdue' only when current_date > make_date(current_year,
current_month, schema.bills.dueDate); if no payment exists for the current month
also mark 'overdue' when current_date > that constructed due date; otherwise
mark 'upcoming'. Use unique symbols schema.payments.paidAt,
schema.bills.dueDate, today.getDate()/current_date and the payments join/month
filter to locate where to update the SQL case expression.
Comment on lines
+34
to
+37
| eq( | ||
| sql`extract('month' from ${schema.payments.forMonthD})`, | ||
| new Date().getMonth() + 1, | ||
| ), |
There was a problem hiding this comment.
Payment filter doesn't account for year boundaries.
Same issue as in dashboard.remote.ts - filtering by month only will match payments from any year with the same month number.
🐛 Proposed fix
eq(
sql`extract('month' from ${schema.payments.forMonthD})`,
new Date().getMonth() + 1,
),
+ eq(
+ sql`extract('year' from ${schema.payments.forMonthD})`,
+ new Date().getFullYear(),
+ ),📝 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
| eq( | |
| sql`extract('month' from ${schema.payments.forMonthD})`, | |
| new Date().getMonth() + 1, | |
| ), | |
| eq( | |
| sql`extract('month' from ${schema.payments.forMonthD})`, | |
| new Date().getMonth() + 1, | |
| ), | |
| eq( | |
| sql`extract('year' from ${schema.payments.forMonthD})`, | |
| new Date().getFullYear(), | |
| ), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 34 - 37, The
month-only filter in the payments query (using eq(sql`extract('month' from
${schema.payments.forMonthD})`, new Date().getMonth() + 1)) will match any year;
update the predicate to also constrain the year by adding a companion check
using extract('year' from ${schema.payments.forMonthD}) === new
Date().getFullYear() (combine both with AND where the eq(...) for month and a
new eq(...) for year are applied) so payments are filtered by both month and
year in the same query.
| paymentId: ulidValidator, | ||
| householdId: ulidValidator, | ||
| proofFile: 'File', | ||
| amount: type("number>=0 | ''").pipe((s) => s === '' && 0), |
There was a problem hiding this comment.
Buggy pipe transform logic.
The expression (s) => s === '' && 0 returns false when s is not an empty string (because s === '' evaluates to false, and false && 0 is false). This means numeric inputs get transformed to false instead of passing through.
🐛 Proposed fix
- amount: type("number>=0 | ''").pipe((s) => s === '' && 0),
+ amount: type("number>=0 | ''").pipe((s) => s === '' ? 0 : s),📝 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
| amount: type("number>=0 | ''").pipe((s) => s === '' && 0), | |
| amount: type("number>=0 | ''").pipe((s) => s === '' ? 0 : s), |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/lib/remotes/payments.remote.ts` at line 67, The pipe on the
amount field incorrectly uses (s) => s === '' && 0 which yields false for
non-empty inputs; update the transform for the amount property so it returns 0
when s is the empty string and otherwise returns the original value (e.g., use a
conditional expression) — locate the amount definition in payments.remote.ts and
replace the current pipe transformer for amount with one that returns s === '' ?
0 : s.
| @@ -4,6 +4,7 @@ import { validateFormData } from '@jhecht/arktype-utils'; | |||
| import { redirect } from '@sveltejs/kit'; | |||
| import { type } from 'arktype'; | |||
| import { and, eq, sql, getTableColumns, asc } from 'drizzle-orm'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check usage of drizzle-orm imports in this file
rg -n "getTableColumns|asc" "apps/website/src/routes/dashboard/+page.server.ts"Repository: sungmanito/mono
Length of output: 126
🏁 Script executed:
#!/bin/bash
# Check usage of the remaining drizzle-orm imports
rg -n "sql|and|eq" "apps/website/src/routes/dashboard/+page.server.ts" | head -20Repository: sungmanito/mono
Length of output: 520
Remove unused drizzle-orm imports getTableColumns and asc.
The imports getTableColumns and asc from line 6 are not used anywhere in this file. The imports sql, and, and eq remain necessary and are used throughout the file (in the household query and addBill action).
Remove getTableColumns and asc from the import statement:
Diff
-import { and, eq, sql, getTableColumns, asc } from 'drizzle-orm';
+import { and, eq, sql } from 'drizzle-orm';📝 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
| import { and, eq, sql, getTableColumns, asc } from 'drizzle-orm'; | |
| import { and, eq, sql } from 'drizzle-orm'; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/routes/dashboard/`+page.server.ts at line 6, Remove the
unused imports getTableColumns and asc from the drizzle-orm import list in the
module (the import that currently includes sql, and, eq, getTableColumns, asc);
edit the import statement so it only imports sql, and, and eq to eliminate the
unused symbols getTableColumns and asc.
| }; | ||
| }; | ||
|
|
||
| $inspect(data.groupings); |
There was a problem hiding this comment.
Remove debug statement before merging.
$inspect is a development/debug tool and should be removed before the code is merged to main.
✏️ Remove debug statement
- $inspect(data.groupings);
-📝 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
| $inspect(data.groupings); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/routes/dashboard/`+page.svelte at line 41, Remove the
development debug call $inspect(data.groupings) from the Svelte page; locate the
expression that references $inspect (the standalone statement using
data.groupings) and delete it so no debug-only inspector runs in production, and
verify there are no remaining unused imports or references to $inspect in the
+page.svelte component (e.g., any top-level $inspect(...) statements).
Comment on lines
+70
to
+77
| let billsWithStatus = createQuery(() => ({ | ||
| queryKey: ['fuckers'], | ||
| queryFn: async () => { | ||
| // Simulate fetching data | ||
| return getUserHouseholdBills(); | ||
| }, | ||
| staleTime: 1000 * 60 * 5, // 5 minutes | ||
| })); |
There was a problem hiding this comment.
Rename queryKey to something descriptive and professional.
The queryKey ['fuckers'] is inappropriate. Use a descriptive key like ['household-bills'] or ['dashboard-bills'].
✏️ Suggested fix
let billsWithStatus = createQuery(() => ({
- queryKey: ['fuckers'],
+ queryKey: ['household-bills'],
queryFn: async () => {
- // Simulate fetching data
return getUserHouseholdBills();
},
staleTime: 1000 * 60 * 5, // 5 minutes
}));📝 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
| let billsWithStatus = createQuery(() => ({ | |
| queryKey: ['fuckers'], | |
| queryFn: async () => { | |
| // Simulate fetching data | |
| return getUserHouseholdBills(); | |
| }, | |
| staleTime: 1000 * 60 * 5, // 5 minutes | |
| })); | |
| let billsWithStatus = createQuery(() => ({ | |
| queryKey: ['household-bills'], | |
| queryFn: async () => { | |
| return getUserHouseholdBills(); | |
| }, | |
| staleTime: 1000 * 60 * 5, // 5 minutes | |
| })); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/routes/dashboard/`+page.svelte around lines 70 - 77, The
queryKey used in the createQuery call for billsWithStatus is inappropriate and
should be renamed to a descriptive, professional key; update the queryKey array
in the createQuery invocation (where billsWithStatus is defined) from
['fuckers'] to a meaningful identifier such as ['household-bills'] or
['dashboard-bills'] so the key reflects the data being fetched and avoids
offensive language.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/website/src/routes/dashboard/payments/+page.server.ts (1)
197-209:⚠️ Potential issue | 🔴 CriticalCritical:
db.transaction()is not awaited - transaction result is discarded.The transaction is fire-and-forget: errors won't propagate to the client, the response returns
{}before the transaction completes, and any rollback or failure is silently lost. The client will receive a success response regardless of whether the database operation succeeded.🐛 Proposed fix
- db.transaction(async (tx) => { + await db.transaction(async (tx) => { try { const res = await makeMultiplePayments( tx, args, locals.supabase, session, ); console.info(res); } catch { tx.rollback(); } }); - return {}; + return { success: true };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/payments/`+page.server.ts around lines 197 - 209, The transaction call to db.transaction is currently fire-and-forget; await the Promise returned by db.transaction when invoking it so the request waits for commit/rollback and errors propagate to the caller, and inside the transaction handler ensure errors are rethrown after calling tx.rollback() so upstream can see the failure; update the block around db.transaction(...) / makeMultiplePayments(tx, args, locals.supabase, session) to await db.transaction(...), catch errors, call tx.rollback() inside the catch, and rethrow the error (or return a rejected promise) rather than swallowing it so the response reflects the actual transaction outcome.
♻️ Duplicate comments (4)
apps/website/src/lib/remotes/images.remote.ts (1)
25-35:⚠️ Potential issue | 🔴 CriticalCritical bug:
idparameter is validated but never used -remove([])deletes nothing.The function validates the ULID parameter but passes an empty array to
remove(), meaning no images are ever deleted. This was previously flagged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/images.remote.ts` around lines 25 - 35, The removeImageByPath handler validates an ULID but never uses it—.remove([]) passes an empty array so nothing is deleted; update removeImageByPath to pass the validated id (or the correct storage path derived from id) into event.locals.supabase.storage.from(PAYMENT_BUCKET_NAME).remove([...]) instead of an empty array, keeping the ulidValidator, using the id variable returned by the command wrapper, and preserving the existing error handling (console.error and error(500,...)) so the storage call actually deletes the intended image.apps/website/src/lib/remotes/dashboard.remote.ts (2)
15-17:⚠️ Potential issue | 🟠 MajorOverdue status calculation only compares day-of-month.
This compares
bills.dueDate(integer 1-31) totoday.getDate()without considering the month context. A bill due on day 30 would show as "upcoming" on the 5th of any month, even if the previous month's payment is overdue. This was previously flagged.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/dashboard.remote.ts` around lines 15 - 17, The status calculation wrongly compares the bill's day-of-month integer (schema.bills.dueDate) to today.getDate(), so replace that day-only comparison with a full date comparison: construct a bill_due_date as a proper date (using the current year/month and falling back to the previous month when the constructed date is in the future) and compare that date to the current date (now()) in the SQL expression used by status (the case expression referencing schema.payments.paidAt and schema.bills.dueDate); update the SQL fragment so it converts the integer day into a full date (handling month rollover) and then uses that full date < now() to determine 'overdue' vs 'upcoming'.
32-41:⚠️ Potential issue | 🟠 MajorPayment join filter missing year - could match payments from wrong year.
The
leftJoinfilters by month number only (extract('month' from forMonthD) = today.getMonth() + 1) without filtering by year. In January 2026, this could incorrectly match January 2025 payments. This was previously flagged.🐛 Proposed fix to include year filter
.leftJoin( schema.payments, and( eq( sql`extract('month' from ${schema.payments.forMonthD})`, today.getMonth() + 1, ), + eq( + sql`extract('year' from ${schema.payments.forMonthD})`, + today.getFullYear(), + ), eq(schema.payments.billId, schema.bills.id), ), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/lib/remotes/dashboard.remote.ts` around lines 32 - 41, The leftJoin filter on payments currently only compares the month via sql`extract('month' from ${schema.payments.forMonthD})` and can match payments from the wrong year; update the join predicate in dashboard.remote.ts (the leftJoin using schema.payments, and(...)) to also compare the year by adding a second sql extract for year (e.g. sql`extract('year' from ${schema.payments.forMonthD})`) equal to today.getFullYear(), alongside the existing month comparison and existing billId equality (schema.payments.billId === schema.bills.id).apps/website/package.json (1)
78-78:⚠️ Potential issue | 🔴 CriticalFix
@tanstack/svelte-queryv6 breaking API change in modalify.svelte.The upgrade to v6 requires all
createQuerycalls to use the thunk (arrow function) pattern. Line 41 ofapps/website/src/lib/components/modalify/modalify.svelteuses the v5 object patterncreateQuery({...})but must be updated tocreateQuery(() => ({...}))to be compatible with v6. This breaking change is required for reactivity in Svelte 5's runes system.The QueryClient options (
refetchOnMount,refetchOnWindowFocus) in svelte-query.ts remain valid in v6.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/package.json` at line 78, Update the createQuery call in modalify.svelte to the v6 thunk pattern: replace the object-style call createQuery({...}) used in Modalify component with a function thunk createQuery(() => ({ ... })) so the query key and fetcher are returned from a function; locate the createQuery invocation in modalify.svelte and wrap its existing object argument in an arrow function, preserving the same key, queryFn and options (refetchOnMount/refetchOnWindowFocus) so it becomes createQuery(() => ({ key, queryFn, ...options })) to satisfy svelte-query v6 and Svelte 5 reactivity.
🧹 Nitpick comments (2)
apps/website/src/routes/dashboard/payments/+page.svelte (1)
63-136: Consider adding an error boundary snippet for failed data fetching.The
svelte:boundaryhas apendingsnippet but nofailedsnippet. IfgetCurrentPayments()throws, the error will propagate up without a user-friendly fallback.💡 Suggested addition
<svelte:boundary> {`#snippet` pending()} {`#each` Array(5)} <div class="card animate-pulse h-16"> </div> {/each} {/snippet} {`#snippet` failed(error)} <div class="card variant-filled-error p-4"> <p>Failed to load payments. Please try again.</p> </div> {/snippet} {`#each` await getCurrentPayments() as payment} ... {/each} </svelte:boundary>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/payments/`+page.svelte around lines 63 - 136, Add a failed snippet to the existing svelte:boundary around the getCurrentPayments() call so users see a friendly error UI when fetching payments fails: implement a {`#snippet` failed(error)} block (paired with the existing {`#snippet` pending()}) that renders a card or message like "Failed to load payments. Please try again." and place it before the {`#each` await getCurrentPayments() as payment} block; this ensures errors thrown by getCurrentPayments() are caught and shown instead of propagating.apps/website/src/routes/dashboard/payments/+page.server.ts (1)
38-39: Remove unprofessional error message.The error log "SHITS FUCKED" is inappropriate for production code and unhelpful for debugging.
♻️ Proposed fix
} catch (e) { - console.error('SHITS FUCKED'); - console.error(e); + console.error('Form validation failed:', e); return fail(400); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/website/src/routes/dashboard/payments/`+page.server.ts around lines 38 - 39, Replace the unprofessional console.error call that prints "SHITS FUCKED" with a clear, professional error log that includes contextual information and the caught error (the variable e)—locate the offending lines using the nearby console.error calls in the handler in +page.server.ts and change the first message to something like "Failed to [action/context]" while keeping the existing console.error(e) (or prefer the app's logging utility if one exists) so the log is both informative and appropriate for production.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/website/src/routes/dashboard/payments/create/`[id=ulid]/+page.svelte:
- Around line 30-37: The current uploadImage.enhance(...) handler intercepts all
submissions from the same form (including the "Unpay" button that uses
formaction="/dashboard/payments?/unpayBill"), causing the uploadImage enhance
logic to run for a distinct server action; split concerns by moving the Unpay
button out of the form that uses uploadImage.enhance and into its own <form>
that targets the unpayBill action (or attach a separate enhance handler for that
form). Locate the uploadImage.enhance(...) call and the Unpay button (the
element with formaction="/dashboard/payments?/unpayBill") and change the markup
so the Unpay control is in a standalone form with its own action/handler to
avoid the uploadImage.enhance interception.
---
Outside diff comments:
In `@apps/website/src/routes/dashboard/payments/`+page.server.ts:
- Around line 197-209: The transaction call to db.transaction is currently
fire-and-forget; await the Promise returned by db.transaction when invoking it
so the request waits for commit/rollback and errors propagate to the caller, and
inside the transaction handler ensure errors are rethrown after calling
tx.rollback() so upstream can see the failure; update the block around
db.transaction(...) / makeMultiplePayments(tx, args, locals.supabase, session)
to await db.transaction(...), catch errors, call tx.rollback() inside the catch,
and rethrow the error (or return a rejected promise) rather than swallowing it
so the response reflects the actual transaction outcome.
---
Duplicate comments:
In `@apps/website/package.json`:
- Line 78: Update the createQuery call in modalify.svelte to the v6 thunk
pattern: replace the object-style call createQuery({...}) used in Modalify
component with a function thunk createQuery(() => ({ ... })) so the query key
and fetcher are returned from a function; locate the createQuery invocation in
modalify.svelte and wrap its existing object argument in an arrow function,
preserving the same key, queryFn and options
(refetchOnMount/refetchOnWindowFocus) so it becomes createQuery(() => ({ key,
queryFn, ...options })) to satisfy svelte-query v6 and Svelte 5 reactivity.
In `@apps/website/src/lib/remotes/dashboard.remote.ts`:
- Around line 15-17: The status calculation wrongly compares the bill's
day-of-month integer (schema.bills.dueDate) to today.getDate(), so replace that
day-only comparison with a full date comparison: construct a bill_due_date as a
proper date (using the current year/month and falling back to the previous month
when the constructed date is in the future) and compare that date to the current
date (now()) in the SQL expression used by status (the case expression
referencing schema.payments.paidAt and schema.bills.dueDate); update the SQL
fragment so it converts the integer day into a full date (handling month
rollover) and then uses that full date < now() to determine 'overdue' vs
'upcoming'.
- Around line 32-41: The leftJoin filter on payments currently only compares the
month via sql`extract('month' from ${schema.payments.forMonthD})` and can match
payments from the wrong year; update the join predicate in dashboard.remote.ts
(the leftJoin using schema.payments, and(...)) to also compare the year by
adding a second sql extract for year (e.g. sql`extract('year' from
${schema.payments.forMonthD})`) equal to today.getFullYear(), alongside the
existing month comparison and existing billId equality (schema.payments.billId
=== schema.bills.id).
In `@apps/website/src/lib/remotes/images.remote.ts`:
- Around line 25-35: The removeImageByPath handler validates an ULID but never
uses it—.remove([]) passes an empty array so nothing is deleted; update
removeImageByPath to pass the validated id (or the correct storage path derived
from id) into
event.locals.supabase.storage.from(PAYMENT_BUCKET_NAME).remove([...]) instead of
an empty array, keeping the ulidValidator, using the id variable returned by the
command wrapper, and preserving the existing error handling (console.error and
error(500,...)) so the storage call actually deletes the intended image.
---
Nitpick comments:
In `@apps/website/src/routes/dashboard/payments/`+page.server.ts:
- Around line 38-39: Replace the unprofessional console.error call that prints
"SHITS FUCKED" with a clear, professional error log that includes contextual
information and the caught error (the variable e)—locate the offending lines
using the nearby console.error calls in the handler in +page.server.ts and
change the first message to something like "Failed to [action/context]" while
keeping the existing console.error(e) (or prefer the app's logging utility if
one exists) so the log is both informative and appropriate for production.
In `@apps/website/src/routes/dashboard/payments/`+page.svelte:
- Around line 63-136: Add a failed snippet to the existing svelte:boundary
around the getCurrentPayments() call so users see a friendly error UI when
fetching payments fails: implement a {`#snippet` failed(error)} block (paired with
the existing {`#snippet` pending()}) that renders a card or message like "Failed
to load payments. Please try again." and place it before the {`#each` await
getCurrentPayments() as payment} block; this ensures errors thrown by
getCurrentPayments() are caught and shown instead of propagating.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: b03949b8-0f28-444f-bf1a-67290bb18789
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (19)
apps/website/package.jsonapps/website/src/lib/components/drawerify/drawerify.svelteapps/website/src/lib/components/monthDropdown/monthDropdown.svelteapps/website/src/lib/remotes/bills.remote.tsapps/website/src/lib/remotes/common.remote.tsapps/website/src/lib/remotes/dashboard.remote.tsapps/website/src/lib/remotes/images.remote.tsapps/website/src/lib/remotes/payments.remote.tsapps/website/src/lib/types.test.tsapps/website/src/lib/typesValidators.tsapps/website/src/lib/util/ark-utils.tsapps/website/src/routes/dashboard/+page.server.tsapps/website/src/routes/dashboard/+page.svelteapps/website/src/routes/dashboard/bills/+page.svelteapps/website/src/routes/dashboard/payments/+page.server.tsapps/website/src/routes/dashboard/payments/+page.svelteapps/website/src/routes/dashboard/payments/create/[id=ulid]/+page.svelteapps/website/svelte.config.jspackage.json
💤 Files with no reviewable changes (1)
- apps/website/src/lib/util/ark-utils.ts
✅ Files skipped from review due to trivial changes (5)
- apps/website/src/routes/dashboard/bills/+page.svelte
- apps/website/svelte.config.js
- apps/website/src/lib/components/monthDropdown/monthDropdown.svelte
- apps/website/src/lib/typesValidators.ts
- apps/website/src/lib/types.test.ts
🚧 Files skipped from review as they are similar to previous changes (7)
- apps/website/src/lib/components/drawerify/drawerify.svelte
- package.json
- apps/website/src/lib/remotes/bills.remote.ts
- apps/website/src/lib/remotes/common.remote.ts
- apps/website/src/routes/dashboard/+page.svelte
- apps/website/src/routes/dashboard/+page.server.ts
- apps/website/src/lib/remotes/payments.remote.ts
Comment on lines
+30
to
+37
| {...uploadImage.enhance(async ({ submit }) => { | ||
| try { | ||
| await submit(); | ||
| onclose(); | ||
| } catch (e) { | ||
| console.error(e); | ||
| } | ||
| })} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Check uploadImage form definition in payments.remote.ts ==="
rg -n -A 30 "export const uploadImage = form" apps/website/src/lib/remotes/payments.remote.ts
echo ""
echo "=== Check if there's an unmarkPayment remote that should be used ==="
rg -n "unmarkPayment" apps/website/src/lib/remotes/payments.remote.tsRepository: sungmanito/mono
Length of output: 1429
🏁 Script executed:
#!/bin/bash
echo "=== View the component to see form and Unpay button structure ==="
sed -n '30,141p' apps/website/src/routes/dashboard/payments/create/[id=ulid]/+page.svelte
echo ""
echo "=== Check for server-side actions in payments routes ==="
find apps/website/src/routes/dashboard/payments -name "+page.server.ts" -o -name "+server.ts" | head -5Repository: sungmanito/mono
Length of output: 3542
🏁 Script executed:
#!/bin/bash
echo "=== Search for unpayBill action definition ==="
rg -n "unpayBill" apps/website/src/routes/dashboard/payments/
echo ""
echo "=== Check payments.server.ts for form actions ==="
rg -n "export const" apps/website/src/routes/dashboard/payments/+page.server.ts 2>/dev/null || echo "File not found or no exports"Repository: sungmanito/mono
Length of output: 423
🏁 Script executed:
#!/bin/bash
echo "=== Check unpayBill action implementation ==="
sed -n '97,120p' apps/website/src/routes/dashboard/payments/+page.server.ts
echo ""
echo "=== Check if there are other buttons in the form that might have different actions ==="
rg -n "type=\"submit\"" apps/website/src/routes/dashboard/payments/create/[id=ulid]/+page.svelteRepository: sungmanito/mono
Length of output: 835
🏁 Script executed:
#!/bin/bash
echo "=== Check the form action in the component ==="
rg -n "<form" apps/website/src/routes/dashboard/payments/create/[id=ulid]/+page.svelte -A 2
echo ""
echo "=== Verify how uploadImage form is exported and used ==="
sed -n '71,90p' apps/website/src/lib/remotes/payments.remote.tsRepository: sungmanito/mono
Length of output: 891
Architectural concern: Multiple form actions mixed in single enhanced form.
The uploadImage.enhance() handler intercepts all form submissions, including the "Unpay" button at line 137-140 which uses formaction="/dashboard/payments?/unpayBill". While SvelteKit's submit() method does respect formaction overrides and will route correctly to the unpayBill action, mixing two distinct server actions (uploadImage and unpayBill) within a single enhanced form is poor separation of concerns.
The enhance handler will process both the primary upload flow and the unpay action unnecessarily through the same handler. Consider moving the Unpay button outside this form into a separate form with its own action handler for cleaner separation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/website/src/routes/dashboard/payments/create/`[id=ulid]/+page.svelte
around lines 30 - 37, The current uploadImage.enhance(...) handler intercepts
all submissions from the same form (including the "Unpay" button that uses
formaction="/dashboard/payments?/unpayBill"), causing the uploadImage enhance
logic to run for a distinct server action; split concerns by moving the Unpay
button out of the form that uses uploadImage.enhance and into its own <form>
that targets the unpayBill action (or attach a separate enhance handler for that
form). Locate the uploadImage.enhance(...) call and the Unpay button (the
element with formaction="/dashboard/payments?/unpayBill") and change the markup
so the Unpay control is in a standalone form with its own action/handler to
avoid the uploadImage.enhance interception.
Contributor
Author
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
1 participant
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.
Summary by CodeRabbit