Chore/misc improvements#142
Merged
Merged
Conversation
- @sveltejs/kit: ^2.47.0 → ^2.55.0 - svelte: ^5.36.5 → ^5.54.1 - vite: ^7.1.10 → ^8.0.1 - @sveltejs/vite-plugin-svelte: ^6.2.1 → ^7.0.0 (requires vite 8) - @sveltejs/adapter-vercel: ^4.0.5 → ^6.3.3 - vitest: ^1.6.1 → ^4.1.0 (required for vite 8 compatibility) Fix CookieSerializeOptions breaking change in hooks.server.ts: @supabase/ssr now requires path to be explicitly set on cookies.set/delete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace data props and TanStack Query with async Svelte pattern. The existing getUserHouseholdBills() and getUserHouseholds() remotes provide all data needed; no new remotes required. - Delete dashboard/+page.server.ts - Wrap dashboard content in <svelte:boundary> with pending skeleton - Derive paidBills/overdueBills/thisWeekBills inside boundary - Replace data.groupings.* with inline-derived filtered arrays - Replace createQuery + billsWithStatus with await getUserHouseholdBills() - Quick Actions use thisWeekBills/overdueBills directly - Add refresh button calling getUserHouseholdBills().refresh() - Fix e.target type cast in hijack attachment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace all bills +page.server.ts files with remote functions in
bills.remote.ts. All bills pages now use async Svelte with
<svelte:boundary> and {#snippet pending()} skeleton loading states.
- Add getBill, getBillsByIds, getBillWithPayments query remotes
- Add createBill, updateBill, updateBills, deleteBills form remotes
- Delete bills +page.server.ts, create, edit, and [id=ulid] server files
- Delete src/lib/server/actions/bills.actions.ts (absorbed into remotes)
- Rewrite bills list, create, detail, single edit, and bulk edit pages
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add getPaymentWithDetails and getPaymentsForIds query remotes to payments.remote.ts. Rewrite all payments pages to use async Svelte with <svelte:boundary> and skeleton loading states. - Add getPaymentWithDetails: fetches payment + Supabase image URL + history - Add getPaymentsForIds: fetches bulk payments by IDs for create page - Delete all 4 payments +page.server.ts files - Delete src/lib/server/actions/payments.actions.ts - Rewrite payments list (improved skeleton), detail, bulk create pages - Rewrite create/[id=ulid] page to use getPayment remote - Replace massPay server action with per-payment uploadImage.enhance forms - Fix svelte:head-inside-svelte:boundary errors in bill and payment detail pages - Fix selectOptions snippet passed as boundary prop in bulk bill edit page Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add households.remote.ts with query/form remotes for all household operations - Replace +layout.server.ts with self-fetching HouseholdSidebar component - Migrate household list, detail, edit, and members pages to async Svelte - Replace TanStack Query (createQuery/createQueries) with remote functions - Replace preloadData + bulkPay with per-payment uploadImage.enhance() forms - Delete 5 +page.server.ts files and invites.action.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tions migration - Remove userHouseholds from App.Locals and hooks.server.ts (no longer pre-fetched per request) - Delete households.actions.ts, images.actions.ts, users.actions.ts (all replaced by remotes) - Export Household type from households.remote.ts; update delete.svelte import - Remove unused households property from getUser() remote return value Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Run prettier across all files - Remove unused imports: command, ulid (bills.remote), form (common.remote), type (common.remote), getUserBills (bill edit page) - Remove dead code: showModal, showMakePaymentModal, CreatePaymentPage import in payments page - Remove unused state: previewUrls in household detail, invites destructuring - Fix unused txResult variable in payments.remote Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- @sveltejs/kit: ^2.47.0 → ^2.55.0 - svelte: ^5.36.5 → ^5.54.1 - vite: ^7.1.10 → ^8.0.1 - @sveltejs/vite-plugin-svelte: ^6.2.1 → ^7.0.0 (requires vite 8) - @sveltejs/adapter-vercel: ^4.0.5 → ^6.3.3 - vitest: ^1.6.1 → ^4.1.0 (required for vite 8 compatibility) Fix CookieSerializeOptions breaking change in hooks.server.ts: @supabase/ssr now requires path to be explicitly set on cookies.set/delete. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace data props and TanStack Query with async Svelte pattern. The existing getUserHouseholdBills() and getUserHouseholds() remotes provide all data needed; no new remotes required. - Delete dashboard/+page.server.ts - Wrap dashboard content in <svelte:boundary> with pending skeleton - Derive paidBills/overdueBills/thisWeekBills inside boundary - Replace data.groupings.* with inline-derived filtered arrays - Replace createQuery + billsWithStatus with await getUserHouseholdBills() - Quick Actions use thisWeekBills/overdueBills directly - Add refresh button calling getUserHouseholdBills().refresh() - Fix e.target type cast in hijack attachment Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace all bills +page.server.ts files with remote functions in
bills.remote.ts. All bills pages now use async Svelte with
<svelte:boundary> and {#snippet pending()} skeleton loading states.
- Add getBill, getBillsByIds, getBillWithPayments query remotes
- Add createBill, updateBill, updateBills, deleteBills form remotes
- Delete bills +page.server.ts, create, edit, and [id=ulid] server files
- Delete src/lib/server/actions/bills.actions.ts (absorbed into remotes)
- Rewrite bills list, create, detail, single edit, and bulk edit pages
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Add getPaymentWithDetails and getPaymentsForIds query remotes to payments.remote.ts. Rewrite all payments pages to use async Svelte with <svelte:boundary> and skeleton loading states. - Add getPaymentWithDetails: fetches payment + Supabase image URL + history - Add getPaymentsForIds: fetches bulk payments by IDs for create page - Delete all 4 payments +page.server.ts files - Delete src/lib/server/actions/payments.actions.ts - Rewrite payments list (improved skeleton), detail, bulk create pages - Rewrite create/[id=ulid] page to use getPayment remote - Replace massPay server action with per-payment uploadImage.enhance forms - Fix svelte:head-inside-svelte:boundary errors in bill and payment detail pages - Fix selectOptions snippet passed as boundary prop in bulk bill edit page Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Add households.remote.ts with query/form remotes for all household operations - Replace +layout.server.ts with self-fetching HouseholdSidebar component - Migrate household list, detail, edit, and members pages to async Svelte - Replace TanStack Query (createQuery/createQueries) with remote functions - Replace preloadData + bulkPay with per-payment uploadImage.enhance() forms - Delete 5 +page.server.ts files and invites.action.ts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…tions migration - Remove userHouseholds from App.Locals and hooks.server.ts (no longer pre-fetched per request) - Delete households.actions.ts, images.actions.ts, users.actions.ts (all replaced by remotes) - Export Household type from households.remote.ts; update delete.svelte import - Remove unused households property from getUser() remote return value Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Run prettier across all files - Remove unused imports: command, ulid (bills.remote), form (common.remote), type (common.remote), getUserBills (bill edit page) - Remove dead code: showModal, showMakePaymentModal, CreatePaymentPage import in payments page - Remove unused state: previewUrls in household detail, invites destructuring - Fix unused txResult variable in payments.remote Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…into chore/misc-improvements
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughThis PR comprehensively migrates the website dashboard from server-side data preloading (via SvelteKit route ChangesServer-to-Client Data Architecture Refactor with Remote Query Consolidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
apps/website/src/lib/components/drawerify/drawerify.svelte (1)
29-37:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFix prop forwarding: don’t spread
...extrasonly into the child component
DrawerifyPropsextendsOmit<DrawerProps, 'children'>, butdrawerify.sveltedestructures Drawer-specific props only foropen/onopen/oncloseand then spreads the remaining...extrasonly into the dynamic<Component>. SinceDrawerconsumesclass(class: propClass) and...restto apply HTML attributes onto its dialog div, Drawer styling/attributes fromDrawerifyprops won’t reach<Drawer>—and Drawer-related props (includingstopScroll) can leak into the child via...extras.
apps/website/src/lib/components/drawerify/drawerify.svelte(lines 29-37, 52-78).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/components/drawerify/drawerify.svelte` around lines 29 - 37, The current implementation spreads the remaining props (extras) only into the dynamic <Component>, causing Drawer-specific props to never reach <Drawer> and leaking drawer props like stopScroll into the child; fix by splitting forwarded props: extract Drawer-related keys (e.g., class/propClass, stopScroll and the rest that Drawer expects) from extras and pass those as drawerProps into the <Drawer> along with open/onopen/onclose, while passing only the child-specific props (e.g., url, loading and remaining non-Drawer keys) into the dynamic Component; update the destructuring around component/open/onopen/onclose and the render so <Drawer> receives its props and the child only receives its intended props.apps/website/src/lib/remotes/payments.remote.ts (2)
149-169:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAbort the mutation when the storage upload fails.
If
uploadToSignedUrl(...)returns an error, this code still falls through and marks the payment paid on Lines 160-169. That leaves the payment updated without the proof image the user just submitted.Suggested fix
const uploadResult = await bucket.uploadToSignedUrl( url.path, url.token, image, ); + if (uploadResult.error) { + error(500, 'Failed to upload image'); + } + if (uploadResult.data) { imageId = (await getImageIdByPath(uploadResult.data.path)) || null; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 149 - 169, The storage upload result (uploadResult from bucket.uploadToSignedUrl) is not being checked for errors, so failures still allow the code to proceed to db.update(schema.payments) and mark the payment as paid; modify the flow to detect uploadResult.error (or a falsy uploadResult.data) immediately after the uploadToSignedUrl call and abort the mutation (throw an appropriate error or return a failed response) instead of continuing, only calling getImageIdByPath and the subsequent db.update when uploadResult indicates success.
245-281:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winAuthorize the payment before deleting its proof object.
The first query fetches a proof image by
paymentIdalone, and the storage delete happens before the ownership check on Lines 273-281. A guessed foreignpaymentIdcan therefore remove another household's proof image even though the payment update itself is rejected.Suggested fix
const [image] = await tx .select() .from(schema.objects) .innerJoin( schema.payments, eq(schema.objects.id, schema.payments.proofImage), ) - .where(eq(schema.payments.id, paymentId)); + .where( + and( + eq(schema.payments.id, paymentId), + inArray( + schema.payments.householdId, + userHouseholds.map((h) => h.id), + ), + ), + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/remotes/payments.remote.ts` around lines 245 - 281, The current flow queries the proof image by paymentId and deletes it from storage before verifying household ownership, allowing deletion of another household's image; fix by authorizing first—either include the household filter (use the same inArray(...) condition against userHouseholds) in the initial tx.select that reads schema.objects joined to schema.payments (paymentId + household constraint) or perform the tx.update with the ownership WHERE and RETURNING proofImage, check the update returned row(s) to confirm the user is authorized, and only then call locals.supabase.storage.from(PAYMENT_BUCKET_NAME).remove([...]) for that proof image; ensure tx.rollback() / error(500, ...) remain used if delete fails.apps/website/src/routes/dashboard/bills/[id=ulid]/+page.svelte (1)
127-159:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't plot missing payment amounts as real values.
Number(p.amount) ?? nullturnsnullinto0andundefinedintoNaN, so unpaid or missing amounts get graphed as actual datapoints.Suggested fix
- data: billData.payments - .slice() - .sort((a, b) => a.forMonthD.getTime() - b.forMonthD.getTime()) - .map((p) => Number(p.amount) ?? null), + data: billData.payments + .slice() + .sort((a, b) => a.forMonthD.getTime() - b.forMonthD.getTime()) + .map((p) => (p.amount == null ? null : Number(p.amount))),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/bills/`[id=ulid]/+page.svelte around lines 127 - 159, The chart is currently converting missing payment amounts into plotted values via Number(p.amount) ?? null; update the mapping used for the dataset in the Chart component (the block that builds data from billData.payments) so that any null/undefined or non-numeric p.amount becomes null (not 0 or NaN) — e.g., check p.amount with p.amount == null or test Number(...) for isNaN and return null in those cases; keep the existing sorting and labels generation unchanged so missing payments render as gaps rather than real datapoints.
🟠 Major comments (17)
.claude/settings.json-14-18 (1)
14-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid destructive/mutating shell permissions in repository-wide settings.
Allowing
git add,git commit, andrmin shared permissions weakens safety boundaries for automated tooling. Keep repo-level permissions read/check-only and move write/delete capabilities to.claude/settings.local.json.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/settings.json around lines 14 - 18, The repository-wide settings file ".claude/settings.json" contains destructive shell permissions like the entries "Bash(git add:*)", "Bash(git commit:*)", and various "Bash(rm ...)" commands; remove these write/delete commands from ".claude/settings.json" so that it only grants read/check-only permissions, and move any needed write/delete entries into ".claude/settings.local.json" (or another local-only config) instead; update the settings to retain non-destructive entries and ensure local-only destructive permissions are documented in the project README or .gitignore so automated tooling uses the local file rather than the shared ".claude/settings.json"..claude/settings.json-12-12 (1)
12-12:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove hard-coded user-specific absolute paths from shared config.
These entries leak local environment details (
C:\Users\Jim\...) and make the repo-level config non-portable across machines/OSes. Use repo-relative patterns or keep machine-specific paths only in local overrides.Proposed fix
- "Bash(ls /c/Users/Jim/projects/bill-tracker/apps/website/src/routes/dashboard/payments/[id=ulid]/)", + "Bash(ls apps/website/src/routes/dashboard/payments/[id=ulid]/)", ... - "Bash(rm \"C:\\\\Users\\\\Jim\\\\projects\\\\bill-tracker\\\\apps\\\\website\\\\src\\\\routes\\\\dashboard\\\\household\\\\[id=ulid]\\\\+page.server.ts\")", - "Bash(rm \"C:\\\\Users\\\\Jim\\\\projects\\\\bill-tracker\\\\apps\\\\website\\\\src\\\\routes\\\\dashboard\\\\household\\\\[id=ulid]\\\\edit\\\\+page.server.ts\")", - "Bash(rm \"C:\\\\Users\\\\Jim\\\\projects\\\\bill-tracker\\\\apps\\\\website\\\\src\\\\routes\\\\dashboard\\\\household\\\\[id=ulid]\\\\members\\\\+page.server.ts\")", + "Bash(rm apps/website/src/routes/dashboard/household/[id=ulid]/+page.server.ts)", + "Bash(rm apps/website/src/routes/dashboard/household/[id=ulid]/edit/+page.server.ts)", + "Bash(rm apps/website/src/routes/dashboard/household/[id=ulid]/members/+page.server.ts)", ... - "c:\\Users\\Jim\\projects\\bill-tracker\\apps\\website\\src\\routes\\dashboard\\bills\\[id=ulid]", - "C:\\Users\\Jim\\projects\\bill-tracker\\apps\\website\\src\\routes\\dashboard\\payments\\[id=ulid]" + "apps/website/src/routes/dashboard/bills/[id=ulid]", + "apps/website/src/routes/dashboard/payments/[id=ulid]"Also applies to: 16-18, 24-25
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/settings.json at line 12, Remove the hard-coded, user-specific absolute path string "Bash(ls /c/Users/Jim/projects/bill-tracker/apps/website/src/routes/dashboard/payments/[id=ulid]/)" from .claude/settings.json and replace it with a portable repo-relative pattern (e.g., use a path rooted at the repo like "Bash(ls apps/website/src/routes/dashboard/payments/[id=ulid]/)" or move the exact machine-specific path into a local-only override), and apply the same change to the other matching entries referenced in the comment (lines with the same C:\Users\Jim\... pattern around 16-18 and 24-25); ensure no user-specific absolute paths remain in the committed repo-level settings.package.json-16-16 (1)
16-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign pnpm version across
package.jsonand GitHub Actions (pnpm/action-setup).
package.jsonsetspackageManagertopnpm@11.2.2+sha512..., but workflows still pin pnpm 9:
.github/workflows/pr.yml/.github/workflows/e2e.yml:pnpm/action-setup@v4withwith.version: 9.0.5.github/workflows/db.yml:pnpm/action-setup@v2withwith.version: 9Update all
pnpm/action-setupwith.versionvalues to11.2.2(or remove explicit pinning) to prevent pnpm version mismatch failures.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 16, package.json's packageManager is set to "pnpm@11.2.2+..." but GitHub Actions use pnpm/action-setup with with.version pinned to 9.x; update the workflow steps that call pnpm/action-setup (the steps that include with.version) to use "11.2.2" (or remove the explicit with.version to let the action pick the packageManager field) so the CI pnpm version matches the packageManager field; search for usages of pnpm/action-setup and the with.version key and replace the version value accordingly.apps/website/src/lib/remotes/households.remote.ts-345-355 (1)
345-355:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't expose a global user-directory search here.
This returns full
schema.usersrows for arbitrary substring input, including email and metadata, to any authenticated caller. That's a pretty broad PII leak for an invite flow.Prefer an exact-email lookup, or at least return only the minimum fields needed for invitation after stricter authorization/rate-limiting.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/remotes/households.remote.ts` around lines 345 - 355, The findUser remote currently returns full schema.users rows for arbitrary substring searches, exposing PII; change it to perform an exact-email lookup and only select minimal fields needed for the invite flow (e.g., id, email, and public display name) instead of selecting the whole users row. Update the where clause in findUser to use an exact equality match against schema.users.email (remove ilike/like substring searches), and replace the top-level db.select().from(schema.users) with a column-limited select that explicitly lists only the safe fields to return.apps/website/src/lib/remotes/bills.remote.ts-136-174 (1)
136-174:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate the batch payloads before indexing the parallel arrays.
Both mutations trust client-controlled
string[]fields and then index them positionally on Lines 168-174 and Lines 283-291. A mismatched submission or a non-numericdue-date/amountturns intoundefined/NaNwrites and either corrupts rows or explodes inside the transaction. Reusing the shared due-date contract here would also keepcreateBillconsistent withupdateBill.Suggested guard
const names = data['name[]']; const householdIdsInput = data['household-id[]']; const dueDates = data['due-date[]'].map(Number); const amounts = data['amount[]'].map(Number); const currencies = data['currency[]']; + const lengths = new Set([ + names.length, + householdIdsInput.length, + dueDates.length, + amounts.length, + currencies.length, + ]); + + if (lengths.size !== 1) { + throw new Error('Mismatched bill form data'); + } + + if (dueDates.some((d) => !Number.isInteger(d) || d < 1 || d > 28)) { + throw new Error('Due date must be between 1 and 28'); + }Apply the same length/range checks before the
updateBillsloop.Also applies to: 255-299
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/remotes/bills.remote.ts` around lines 136 - 174, The createBill handler is indexing client-controlled parallel arrays (names, household-id[], due-date[], amount[], currency[]) without validating their lengths or numeric parsing, which can produce undefined/NaN values and corrupt the DB; before entering the transaction in createBill (and likewise before the updateBills loop in the update handler), assert that all arrays have the same length, that due-date and amount entries parse to finite numbers (use Number(...) and Number.isFinite), and enforce range/positivity rules (e.g., amount > 0 or set undefined) — reuse the existing due-date/amount validation logic/contract (billCreateValidator or whatever due-date validator exists) to perform these checks and throw a clear error if validation fails so you never build the values array with mismatched or non-numeric entries.apps/website/src/lib/remotes/households.remote.ts-308-318 (1)
308-318:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid destructuring an empty ownership query.
const [{ isOwner }] = ...throws before your!isOwnerbranch runs when the household id is stale or invalid. That turns a normal authorization/not-found path into a 500.Suggested fix
- const [{ isOwner }] = await tx + const [row] = await tx .select({ isOwner: sql<boolean>`${schema.households.ownerId} = ${user.id}`, }) .from(schema.households) .where(eq(schema.households.id, data['household-id'])); - if (!isOwner) { + if (!row?.isOwner) { tx.rollback(); return false; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/remotes/households.remote.ts` around lines 308 - 318, The ownership query currently destructures directly with "const [{ isOwner }]" which throws if no rows are returned; change the tx select in the db.transaction block to capture the result array (e.g. const rows = await tx.select(...).from(schema.households).where(...)), check if rows.length === 0 and return false (or handle not-found) before accessing rows[0].isOwner, then proceed with the existing authorization check and rollback logic; this ensures stale/invalid household ids don't cause an exception.apps/website/src/lib/remotes/bills.remote.ts-67-107 (1)
67-107:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReturn a 404 when the bill lookup misses.
If this lookup returns no row, Line 107 still returns
{ payments }and silently drops every bill field. Callers then fail later with a malformed object instead of getting a clean not-found response.Suggested fix
+import { error } from '`@sveltejs/kit`'; + export const getBillWithPayments = query(ulidValidator, async (id) => { const userHouseholds = await getUserHouseholds(); const [bill, payments] = await Promise.all([ @@ }), ]); + if (!bill) error(404, 'Bill not found'); + return { ...bill, payments }; });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/remotes/bills.remote.ts` around lines 67 - 107, In getBillWithPayments, if the bills query returns no row you currently return { payments } which produces a malformed object; after the Promise.all that resolves [bill, payments] check whether bill is null/undefined and if so return or throw an HTTP 404 (not-found) error (include the bill id in the message for diagnostics) instead of continuing to return the payments-only object so callers receive a proper not-found response. Use the surrounding API/endpoint error mechanism (e.g., throw a 404 Response or framework-specific notFound helper) inside getBillWithPayments to implement this.apps/website/src/routes/dashboard/payments/create/[id=ulid]/+page.svelte-40-66 (1)
40-66:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard the missing-payment path before dereferencing
payment.
getPayment(id)can resolve toundefinedhere — the remote currently returnsr[0]inapps/website/src/lib/remotes/payments.remote.ts:197-228. A stale or unauthorized ULID will then blow up on Line 65 instead of returning a 404/not-found state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/payments/create/`[id=ulid]/+page.svelte around lines 40 - 66, The page currently dereferences payment (payment.id, payment.householdId) after awaiting getPayment(id); add a guard right after {`@const` payment = await getPayment(id)} to handle payment === undefined (or null) by rendering a not-found/404 state or returning early (e.g., show a “Payment not found” message and do not render the form or call uploadImage.enhance). Ensure the check references the same symbols (payment and getPayment) and prevents any use of payment later in the form (inputs and any logic that calls onclose/uploadImage.enhance) so stale/unauthorized ULIDs don’t throw.apps/website/src/routes/dashboard/payments/create/[id=ulid]/+page.svelte-117-119 (1)
117-119:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the preview-clear button non-submitting.
Inside a form,
<button>defaults to submit. Clicking the X currently clears the preview and submitsuploadImage, which can mark the payment paid unintentionally. Addtype="button".Suggested fix
- <button onclick={() => (file = null)}> + <button type="button" onclick={() => (file = null)}> <XIcon size="1em" /> </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/payments/create/`[id=ulid]/+page.svelte around lines 117 - 119, The preview-clear button inside the payment form is submitting the form by default; change the button in the component that renders the preview-clear control (the button that currently does onclick={() => (file = null)} and shows <XIcon />) to be non-submitting by adding type="button" so clicking it only clears the local file variable and does not trigger the uploadImage/form submission.apps/website/src/routes/dashboard/+page.svelte-375-396 (1)
375-396:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFilter these batch-payment IDs before building the URL.
Pay This Weekcurrently includes everyupcomingbill, not bills due in the next 7 days, andPay Overduecan emitpayments[]=undefinedfor overdue bills without a joined payment row. That makes the action label inaccurate and can send invalid IDs into/dashboard/payments/create.Suggested fix
<button class="w-full mb-3 py-2 rounded-xl bg-yellow-100 text-yellow-800 font-semibold flex items-center gap-2 justify-center" onclick={() => { - const ids = billsByStatus.data.upcoming - .filter((bill) => bill.payment !== null) - .map((b) => `payments[]=${b.payment?.id}`) + const now = new Date(); + const weekFromNow = new Date(); + weekFromNow.setDate(now.getDate() + 7); + + const ids = billsByStatus.data.upcoming + .flatMap((bill) => { + if (!bill.payment?.id) return []; + const dueDate = new Date(bill.dueDate); + return dueDate >= now && dueDate <= weekFromNow + ? [`payments[]=${bill.payment.id}`] + : []; + }) .join('&'); makeMultiplePayments.url = `/dashboard/payments/create?${ids}`; makeMultiplePayments.show = true; }} > @@ <button class="w-full mb-3 py-2 rounded-xl bg-red-100 text-red-700 font-semibold flex items-center gap-2 justify-center" onclick={() => { const ids = billsByStatus.data.overdue - .map((bill) => `payments[]=${bill.payment?.id}`) + .flatMap((bill) => + bill.payment?.id ? [`payments[]=${bill.payment.id}`] : [], + ) .join('&'); makeMultiplePayments.url = `/dashboard/payments/create?${ids}`; makeMultiplePayments.show = true; }} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/`+page.svelte around lines 375 - 396, The onclick handlers for Pay This Week and Pay Overdue should filter bills to only include those with an actual payment id and, for Pay This Week, only bills due within the next 7 days; in other words, update the handlers that reference billsByStatus.data.upcoming and billsByStatus.data.overdue and replace the current .filter/.map pipeline so you: 1) exclude bills where bill.payment is null or bill.payment.id is undefined, 2) for the upcoming case also check bill.dueDate (or the appropriate date field) falls within 7 days from now, then map to `payments[]=<id>` and join('&') before assigning makeMultiplePayments.url and showing makeMultiplePayments; also avoid building the URL or calling show if the resulting ids string is empty.apps/website/src/routes/dashboard/household/[id=ulid]/+page.svelte-168-172 (1)
168-172:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove submitted IDs from
selectedPayments.After
submit()succeeds, the paid payment ID stays selected. Once the refresh flips that bill to paid, its checkbox disappears but the “Mark N as Paid” action stays enabled, so the drawer can reopen with already-paid entries and allow duplicate submissions.Suggested fix
{...uploadImage.enhance(async ({ submit }) => { await submit(); + selectedPayments = selectedPayments.filter( + (id) => id !== payment.id, + ); getUserBills().refresh(); getHouseholdDetail(page.params.id).refresh(); })}Also applies to: 313-317
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/household/`[id=ulid]/+page.svelte around lines 168 - 172, After a successful uploadImage.enhance submit(), remove any payment IDs that were just submitted from the selectedPayments set so the "Mark N as Paid" action is disabled for already-paid entries; specifically, in the uploadImage.enhance success handler (the async ({ submit }) => { await submit(); ... }) call selectedPayments.delete(id) or filter out submitted IDs before calling getUserBills().refresh() and getHouseholdDetail(page.params.id).refresh(); apply the same removal logic in the other identical handler around lines 313-317 so the drawer cannot reopen with already-paid entries.apps/website/src/routes/dashboard/household/[id=ulid]/edit/+page.svelte-70-76 (1)
70-76:⚠️ Potential issue | 🟠 MajorSet the Close button to
type="button"to prevent accidental form submission
Buttondoesn’t set a nativetype, so inside the<form>the Close button defaults totype="submit". Clicking Close can submit the form (triggeringupdateHousehold.enhance/submit()) and invokeonclose()both from the click handler and again aftersubmit()whencomponentis true. Settype="button"on the Close button (optionally also settype="submit"on Save for clarity).
apps/website/src/routes/dashboard/household/[id=ulid]/edit/+page.svelte (lines ~70-77)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/household/`[id=ulid]/edit/+page.svelte around lines 70 - 76, The Close Button inside the form defaults to type="submit" causing accidental form submissions; update the Close <Button> (the instance using onclick={() => onclose()} and disabled={saving}) to include type="button" to prevent submission, and optionally set the Save <Button> to type="submit" for clarity so form handling (updateHousehold.enhance/submit) only runs on the intended action.apps/website/src/routes/dashboard/bills/edit/[ids=ulids]/+page.svelte-99-105 (1)
99-105:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve unset amounts instead of forcing
0.
value={bill.amount || 0}serializes missing amounts as0, and the currentupdateBillsremote writes submitted numbers directly. Editing a bill with no amount will silently overwrite it to zero.A paired fix is needed here and in
apps/website/src/lib/remotes/bills.remote.ts:255-308: render''for unset amounts, then treat blankbills[].amountvalues as optional instead of coercing them withNumber('').🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/bills/edit/`[ids=ulids]/+page.svelte around lines 99 - 105, The input currently forces missing amounts to 0 via value={bill.amount || 0}; change the UI to render an empty string for unset amounts (e.g., use value={bill.amount ?? ''} or otherwise render '' when bill.amount is null/undefined) for the input named "bills[].amount" so blanks are submitted as empty strings; then update the updateBills handler in bills.remote.ts (the updateBills function that parses incoming bills) to treat blank strings for bills[].amount as absent instead of coercing with Number('') — parse non-empty values to numbers but leave empty/blank values as undefined/null so existing unset amounts are preserved on save.apps/website/src/routes/dashboard/bills/[id=ulid]/+page.svelte-63-85 (1)
63-85:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGuard
billDatabefore dereferencing it.
getBillWithPayments(id)is built fromr[0]inapps/website/src/lib/remotes/bills.remote.ts:67-108, so a stale or unauthorized id can resolve without a bill. This block immediately readsbillData.household.name,billData.billName, etc., which will crash the boundary instead of rendering a controlled not-found state.Suggested fix
{`@const` billData = await getBillWithPayments(id)} - <div class="p-6 flex-grow `@container`"> + {`#if` !billData?.id} + <div class="p-6 flex-grow `@container`"> + <div class="wrapper `@3xl`:max-w-[75vw] mx-auto"> + <Alert type="warning">Bill not found.</Alert> + </div> + </div> + {:else} + <div class="p-6 flex-grow `@container`"> <div class="wrapper `@3xl`:max-w-[75vw] mx-auto"> ... </div> </div> + {/if}Also applies to: 95-120
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/bills/`[id=ulid]/+page.svelte around lines 63 - 85, Guard against null/undefined billData returned by getBillWithPayments(id) before accessing properties; update the +page.svelte block that uses billData (the const billData = await getBillWithPayments(id) and the subsequent Breadcrumb and Header usage) to check if billData exists and either render a controlled "not found" UI (or call SvelteKit's error(404) / return a 404 response) in the else branch, so you don't dereference billData.household.name or billData.billName when the remote returned no bill.apps/website/src/routes/dashboard/bills/+page.svelte-103-112 (1)
103-112:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKey the household buckets by
householdId, nothouseholdName.This reducer collapses different households when they share the same display name. Since the header checkbox/edit/delete actions operate on each bucket, two unrelated households with the same
householdNamewill be bulk-managed together.Suggested fix
- {`@const` byHousehold = bills.reduce( - (acc, bill) => { - if (!acc[bill.householdName]) { - acc[bill.householdName] = []; - } - acc[bill.householdName].push(bill); - return acc; - }, - {} as Record<string, (typeof bills)[number][]>, - )} + {`@const` byHousehold = bills.reduce( + (acc, bill) => { + if (!acc[bill.householdId]) { + acc[bill.householdId] = { + householdName: bill.householdName, + bills: [], + }; + } + acc[bill.householdId].bills.push(bill); + return acc; + }, + {} as Record< + string, + { householdName: string; bills: (typeof bills)[number][] } + >, + )} ... - {`#each` Object.entries(byHousehold) as [householdName, householdBills]} + {`#each` Object.values(byHousehold) as { householdName, bills: householdBills }}Also applies to: 207-245
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/bills/`+page.svelte around lines 103 - 112, The reducer that builds byHousehold is keying buckets by bill.householdName which can merge distinct households that share a display name; update the reduce call used to create byHousehold (and the analogous grouping code around where the same pattern is repeated at lines ~207-245) to use bill.householdId as the Record key and keep householdName only for display, i.e., change any access/creation of acc[bill.householdName] to acc[bill.householdId] and adjust types and any downstream uses (headers/checkbox/edit/delete handlers) to reference the householdId key while still reading householdName for labels.apps/website/src/routes/dashboard/bills/create/+page.svelte-59-64 (1)
59-64:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle the standalone success path.
This always calls
onclose(), butoncloseis a no-op when the page is rendered as a normal route. A successful standalone create will leave the user on the same form with no navigation or reset.Suggested fix
+ import { goto } from '$app/navigation'; ... <form {...createBill.enhance(async ({ submit }) => { await submit(); getUserBills().refresh(); - onclose(); + if (component) { + onclose(); + } else { + goto('/dashboard/bills'); + } })}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/bills/create/`+page.svelte around lines 59 - 64, The form's enhance callback always calls onclose() even when the page is used as a standalone route (where onclose is a no-op), so detect the action result and only call onclose on a true dialog/embedded success; instead, inspect the enhance callback args (use the result returned from submit or the enhance callback parameter, e.g. check result?.type === 'success' or use the returned value from submit()) and: call getUserBills().refresh() on success, then if typeof onclose === 'function' call onclose(), else (standalone) reset the form fields and/or navigate or display a success message so the user isn't left on the filled form; update the createBill.enhance handler to branch on result success and check onclose before invoking it.apps/website/src/routes/dashboard/bills/[id=ulid]/edit/+page.svelte-57-61 (1)
57-61:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMake the header close button non-submitting.
The close
<button>is rendered inside the<form>; withouttype="button", it defaults totype="submit", so clicking X can submit the update instead of just closing.Suggested fix
- <button onclick={() => onclose()}> + <button type="button" onclick={() => onclose()}> <XIcon size="1em" /> </button>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/bills/`[id=ulid]/edit/+page.svelte around lines 57 - 61, The close button inside the form (rendered in the {`#snippet` actions()} block where component is truthy) currently lacks an explicit type and therefore defaults to type="submit", causing clicks to submit the form; update the button used to render the XIcon (the button that calls onclose()) to be non-submitting by adding type="button" to the element so it only triggers onclose() and does not submit the form.
🟡 Minor comments (3)
apps/website/src/lib/components/card/card.stories.svelte-23-26 (1)
23-26:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReplace the placeholder docs text with professional copy.
Line 25 currently uses inappropriate text for Storybook docs.
Suggested fix
description: { - story: 'your mother', + story: 'Basic card container with optional header and footer snippets.', },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/components/card/card.stories.svelte` around lines 23 - 26, Replace the inappropriate placeholder string used in the Storybook docs for the Card component: update the docs.description.story value (currently 'your mother') in card.stories.svelte to a professional, descriptive sentence that summarizes the story purpose (e.g., describing what the Card demonstrates, props showcased, and intended usage). Locate the docs block that sets description.story and replace the text with concise, professional copy such as "Demonstrates the Card component with title, image, and actions; use this story to preview props and layout variants."apps/website/src/lib/components/drawer/drawer.svelte-72-74 (1)
72-74:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove debug logging from Escape handling.
Line 73 logs every Escape key event and should not ship in shared UI primitives.
Suggested fix
if (e.key === 'Escape') { - console.info('YO WTF', e); dispatchCloseEvent(); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/lib/components/drawer/drawer.svelte` around lines 72 - 74, Remove the stray debug console logging in the Escape key handler: inside the keydown handler where it checks if (e.key === 'Escape') and calls dispatchCloseEvent(), delete the console.info('YO WTF', e) statement so only dispatchCloseEvent() is executed; ensure no other debug logs remain in the same handler or related functions in drawer.svelte.apps/website/src/routes/dashboard/+page.svelte-155-177 (1)
155-177:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse a real count for the third summary card.
value: billsByStatus.data.upcomingpasses the whole array into the card, so this will render an object string instead of a numeric metric. Also, the upstream query inapps/website/src/lib/remotes/dashboard.remote.ts:7-47only bucketspaid | overdue | upcoming, so this is not actually “Due This Week” yet. Either rename this card to “Upcoming” and use.length, or derive a week-scoped bucket first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/`+page.svelte around lines 155 - 177, The third summary card is passing the entire upcoming array (billsByStatus.data.upcoming) instead of a numeric count and is labeled “Due This Week” even though the remote only buckets as "upcoming"; update the summary constant so the third item uses value: billsByStatus.data.upcoming.length and either change label to "Upcoming" or implement a week-scoped filter before assigning value (e.g., derive a weekFilteredUpcoming array from billsByStatus.data.upcoming and use its .length) to keep label and data semantically correct.
🧹 Nitpick comments (1)
.claude/settings.local.json (1)
7-8: ⚡ Quick winNarrow wildcard command permissions for least privilege.
Bash(cat *)andBash(ls *)are very broad. Prefer scoped path patterns (for example, only project subdirs or specific file globs) to reduce accidental data exposure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.claude/settings.local.json around lines 7 - 8, The current broad permissions "Bash(cat *)" and "Bash(ls *)" should be narrowed to least-privilege scoped globs; replace those entries with path-limited patterns that only allow required directories/files (e.g., "Bash(cat project/*)" or "Bash(cat src/**/*.js)" and "Bash(ls project/*)") or explicit file lists for the specific tools/data the agent needs, updating the entries in the .claude/settings.local.json array where "Bash(cat *)" and "Bash(ls *)" appear.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/website/src/lib/attachments/command.svelte.ts`:
- Around line 37-47: The code mutates a raw Promise by assigning b.something,
which TypeScript rejects; update the remote factory to return a wrapper object
instead (e.g., define a small interface/type like RemoteResult with a promise:
Promise<number> and something: () => void, create the Promise as before into a
local variable b and return { promise: b, something: () => { ... } }) or
alternatively return an object with the promise under a property and the
something function alongside it; also update any callers that expect a Promise
to use result.promise (or adapt types) so you don't add ad-hoc properties to the
Promise itself.
- Around line 18-34: The submit handler is incomplete and the listener cleanup
mismatches capture mode: update onSubmit to call e.preventDefault(), then create
the FormData from the form, convert it via formDataToObject (fd and obj) and
invoke the remote command/arg with the resulting obj (await if it returns a
Promise); move fd/obj creation inside onSubmit so they aren’t unused. Also
ensure you add and remove the listener with the same capture flag (use true in
both form.addEventListener('submit', onSubmit, true) and
form.removeEventListener('submit', onSubmit, true)). Use the existing names
onSubmit, formDataToObject, fd/obj, form.addEventListener and
form.removeEventListener to locate the code.
In `@apps/website/src/lib/remotes/households.remote.ts`:
- Around line 451-459: The claimHousehold handler currently lets any
authenticated user overwrite ownerId; before updating in claimHousehold (which
calls getUser and db.update(schema.households)), enforce an authorization
predicate: only allow claiming if the household is unowned (ownerId IS NULL) or
otherwise permitted. Fix by either SELECTing the household first and rejecting
if household.ownerId is non-null, or include the predicate in the UPDATE .where
clause using schema.households.ownerId (e.g. require IS NULL or an allowed-owner
check) and return a 403/validation error when the predicate fails.
In `@apps/website/src/routes/dashboard/bills/`+page.svelte:
- Around line 152-163: The Drawerify is mounting the wrong component for
showBillDetailsStore.url: currently it passes ShowBillDetailsComponent (which
renders the batch-edit UI) instead of the bill-details page component for the
/dashboard/bills/:billId route; update the component prop on Drawerify to the
correct bill details component used by that route (the page/component that
renders a single bill detail), ensure it receives the same props shape (data,
component, onclose) and keep the existing bind:open,
url={showBillDetailsStore.url}, and onclose behavior.
In `@apps/website/src/routes/dashboard/bills/edit/`[ids=ulids]/+page.svelte:
- Around line 18-20: The ids derivation currently uses idsProp ||
page.params.id... which always picks the default empty idsProp and also
references the wrong route param name; update the computation for ids (the
$derived call that computes ids from idsProp and page.params) to treat an empty
idsProp as absent (e.g., check idsProp.length or use a nullish/length-aware
check) and read page.params.ids (not page.params.id) as the fallback source;
modify the code around the ids variable (and the Props destructuring that
defines idsProp) so that IDs parse correctly from the route when idsProp is
empty.
---
Outside diff comments:
In `@apps/website/src/lib/components/drawerify/drawerify.svelte`:
- Around line 29-37: The current implementation spreads the remaining props
(extras) only into the dynamic <Component>, causing Drawer-specific props to
never reach <Drawer> and leaking drawer props like stopScroll into the child;
fix by splitting forwarded props: extract Drawer-related keys (e.g.,
class/propClass, stopScroll and the rest that Drawer expects) from extras and
pass those as drawerProps into the <Drawer> along with open/onopen/onclose,
while passing only the child-specific props (e.g., url, loading and remaining
non-Drawer keys) into the dynamic Component; update the destructuring around
component/open/onopen/onclose and the render so <Drawer> receives its props and
the child only receives its intended props.
In `@apps/website/src/lib/remotes/payments.remote.ts`:
- Around line 149-169: The storage upload result (uploadResult from
bucket.uploadToSignedUrl) is not being checked for errors, so failures still
allow the code to proceed to db.update(schema.payments) and mark the payment as
paid; modify the flow to detect uploadResult.error (or a falsy
uploadResult.data) immediately after the uploadToSignedUrl call and abort the
mutation (throw an appropriate error or return a failed response) instead of
continuing, only calling getImageIdByPath and the subsequent db.update when
uploadResult indicates success.
- Around line 245-281: The current flow queries the proof image by paymentId and
deletes it from storage before verifying household ownership, allowing deletion
of another household's image; fix by authorizing first—either include the
household filter (use the same inArray(...) condition against userHouseholds) in
the initial tx.select that reads schema.objects joined to schema.payments
(paymentId + household constraint) or perform the tx.update with the ownership
WHERE and RETURNING proofImage, check the update returned row(s) to confirm the
user is authorized, and only then call
locals.supabase.storage.from(PAYMENT_BUCKET_NAME).remove([...]) for that proof
image; ensure tx.rollback() / error(500, ...) remain used if delete fails.
In `@apps/website/src/routes/dashboard/bills/`[id=ulid]/+page.svelte:
- Around line 127-159: The chart is currently converting missing payment amounts
into plotted values via Number(p.amount) ?? null; update the mapping used for
the dataset in the Chart component (the block that builds data from
billData.payments) so that any null/undefined or non-numeric p.amount becomes
null (not 0 or NaN) — e.g., check p.amount with p.amount == null or test
Number(...) for isNaN and return null in those cases; keep the existing sorting
and labels generation unchanged so missing payments render as gaps rather than
real datapoints.
---
Major comments:
In @.claude/settings.json:
- Around line 14-18: The repository-wide settings file ".claude/settings.json"
contains destructive shell permissions like the entries "Bash(git add:*)",
"Bash(git commit:*)", and various "Bash(rm ...)" commands; remove these
write/delete commands from ".claude/settings.json" so that it only grants
read/check-only permissions, and move any needed write/delete entries into
".claude/settings.local.json" (or another local-only config) instead; update the
settings to retain non-destructive entries and ensure local-only destructive
permissions are documented in the project README or .gitignore so automated
tooling uses the local file rather than the shared ".claude/settings.json".
- Line 12: Remove the hard-coded, user-specific absolute path string "Bash(ls
/c/Users/Jim/projects/bill-tracker/apps/website/src/routes/dashboard/payments/[id=ulid]/)"
from .claude/settings.json and replace it with a portable repo-relative pattern
(e.g., use a path rooted at the repo like "Bash(ls
apps/website/src/routes/dashboard/payments/[id=ulid]/)" or move the exact
machine-specific path into a local-only override), and apply the same change to
the other matching entries referenced in the comment (lines with the same
C:\Users\Jim\... pattern around 16-18 and 24-25); ensure no user-specific
absolute paths remain in the committed repo-level settings.
In `@apps/website/src/lib/remotes/bills.remote.ts`:
- Around line 136-174: The createBill handler is indexing client-controlled
parallel arrays (names, household-id[], due-date[], amount[], currency[])
without validating their lengths or numeric parsing, which can produce
undefined/NaN values and corrupt the DB; before entering the transaction in
createBill (and likewise before the updateBills loop in the update handler),
assert that all arrays have the same length, that due-date and amount entries
parse to finite numbers (use Number(...) and Number.isFinite), and enforce
range/positivity rules (e.g., amount > 0 or set undefined) — reuse the existing
due-date/amount validation logic/contract (billCreateValidator or whatever
due-date validator exists) to perform these checks and throw a clear error if
validation fails so you never build the values array with mismatched or
non-numeric entries.
- Around line 67-107: In getBillWithPayments, if the bills query returns no row
you currently return { payments } which produces a malformed object; after the
Promise.all that resolves [bill, payments] check whether bill is null/undefined
and if so return or throw an HTTP 404 (not-found) error (include the bill id in
the message for diagnostics) instead of continuing to return the payments-only
object so callers receive a proper not-found response. Use the surrounding
API/endpoint error mechanism (e.g., throw a 404 Response or framework-specific
notFound helper) inside getBillWithPayments to implement this.
In `@apps/website/src/lib/remotes/households.remote.ts`:
- Around line 345-355: The findUser remote currently returns full schema.users
rows for arbitrary substring searches, exposing PII; change it to perform an
exact-email lookup and only select minimal fields needed for the invite flow
(e.g., id, email, and public display name) instead of selecting the whole users
row. Update the where clause in findUser to use an exact equality match against
schema.users.email (remove ilike/like substring searches), and replace the
top-level db.select().from(schema.users) with a column-limited select that
explicitly lists only the safe fields to return.
- Around line 308-318: The ownership query currently destructures directly with
"const [{ isOwner }]" which throws if no rows are returned; change the tx select
in the db.transaction block to capture the result array (e.g. const rows = await
tx.select(...).from(schema.households).where(...)), check if rows.length === 0
and return false (or handle not-found) before accessing rows[0].isOwner, then
proceed with the existing authorization check and rollback logic; this ensures
stale/invalid household ids don't cause an exception.
In `@apps/website/src/routes/dashboard/`+page.svelte:
- Around line 375-396: The onclick handlers for Pay This Week and Pay Overdue
should filter bills to only include those with an actual payment id and, for Pay
This Week, only bills due within the next 7 days; in other words, update the
handlers that reference billsByStatus.data.upcoming and
billsByStatus.data.overdue and replace the current .filter/.map pipeline so you:
1) exclude bills where bill.payment is null or bill.payment.id is undefined, 2)
for the upcoming case also check bill.dueDate (or the appropriate date field)
falls within 7 days from now, then map to `payments[]=<id>` and join('&') before
assigning makeMultiplePayments.url and showing makeMultiplePayments; also avoid
building the URL or calling show if the resulting ids string is empty.
In `@apps/website/src/routes/dashboard/bills/`[id=ulid]/+page.svelte:
- Around line 63-85: Guard against null/undefined billData returned by
getBillWithPayments(id) before accessing properties; update the +page.svelte
block that uses billData (the const billData = await getBillWithPayments(id) and
the subsequent Breadcrumb and Header usage) to check if billData exists and
either render a controlled "not found" UI (or call SvelteKit's error(404) /
return a 404 response) in the else branch, so you don't dereference
billData.household.name or billData.billName when the remote returned no bill.
In `@apps/website/src/routes/dashboard/bills/`[id=ulid]/edit/+page.svelte:
- Around line 57-61: The close button inside the form (rendered in the {`#snippet`
actions()} block where component is truthy) currently lacks an explicit type and
therefore defaults to type="submit", causing clicks to submit the form; update
the button used to render the XIcon (the button that calls onclose()) to be
non-submitting by adding type="button" to the element so it only triggers
onclose() and does not submit the form.
In `@apps/website/src/routes/dashboard/bills/`+page.svelte:
- Around line 103-112: The reducer that builds byHousehold is keying buckets by
bill.householdName which can merge distinct households that share a display
name; update the reduce call used to create byHousehold (and the analogous
grouping code around where the same pattern is repeated at lines ~207-245) to
use bill.householdId as the Record key and keep householdName only for display,
i.e., change any access/creation of acc[bill.householdName] to
acc[bill.householdId] and adjust types and any downstream uses
(headers/checkbox/edit/delete handlers) to reference the householdId key while
still reading householdName for labels.
In `@apps/website/src/routes/dashboard/bills/create/`+page.svelte:
- Around line 59-64: The form's enhance callback always calls onclose() even
when the page is used as a standalone route (where onclose is a no-op), so
detect the action result and only call onclose on a true dialog/embedded
success; instead, inspect the enhance callback args (use the result returned
from submit or the enhance callback parameter, e.g. check result?.type ===
'success' or use the returned value from submit()) and: call
getUserBills().refresh() on success, then if typeof onclose === 'function' call
onclose(), else (standalone) reset the form fields and/or navigate or display a
success message so the user isn't left on the filled form; update the
createBill.enhance handler to branch on result success and check onclose before
invoking it.
In `@apps/website/src/routes/dashboard/bills/edit/`[ids=ulids]/+page.svelte:
- Around line 99-105: The input currently forces missing amounts to 0 via
value={bill.amount || 0}; change the UI to render an empty string for unset
amounts (e.g., use value={bill.amount ?? ''} or otherwise render '' when
bill.amount is null/undefined) for the input named "bills[].amount" so blanks
are submitted as empty strings; then update the updateBills handler in
bills.remote.ts (the updateBills function that parses incoming bills) to treat
blank strings for bills[].amount as absent instead of coercing with Number('') —
parse non-empty values to numbers but leave empty/blank values as undefined/null
so existing unset amounts are preserved on save.
In `@apps/website/src/routes/dashboard/household/`[id=ulid]/+page.svelte:
- Around line 168-172: After a successful uploadImage.enhance submit(), remove
any payment IDs that were just submitted from the selectedPayments set so the
"Mark N as Paid" action is disabled for already-paid entries; specifically, in
the uploadImage.enhance success handler (the async ({ submit }) => { await
submit(); ... }) call selectedPayments.delete(id) or filter out submitted IDs
before calling getUserBills().refresh() and
getHouseholdDetail(page.params.id).refresh(); apply the same removal logic in
the other identical handler around lines 313-317 so the drawer cannot reopen
with already-paid entries.
In `@apps/website/src/routes/dashboard/household/`[id=ulid]/edit/+page.svelte:
- Around line 70-76: The Close Button inside the form defaults to type="submit"
causing accidental form submissions; update the Close <Button> (the instance
using onclick={() => onclose()} and disabled={saving}) to include type="button"
to prevent submission, and optionally set the Save <Button> to type="submit" for
clarity so form handling (updateHousehold.enhance/submit) only runs on the
intended action.
In `@apps/website/src/routes/dashboard/payments/create/`[id=ulid]/+page.svelte:
- Around line 40-66: The page currently dereferences payment (payment.id,
payment.householdId) after awaiting getPayment(id); add a guard right after
{`@const` payment = await getPayment(id)} to handle payment === undefined (or
null) by rendering a not-found/404 state or returning early (e.g., show a
“Payment not found” message and do not render the form or call
uploadImage.enhance). Ensure the check references the same symbols (payment and
getPayment) and prevents any use of payment later in the form (inputs and any
logic that calls onclose/uploadImage.enhance) so stale/unauthorized ULIDs don’t
throw.
- Around line 117-119: The preview-clear button inside the payment form is
submitting the form by default; change the button in the component that renders
the preview-clear control (the button that currently does onclick={() => (file =
null)} and shows <XIcon />) to be non-submitting by adding type="button" so
clicking it only clears the local file variable and does not trigger the
uploadImage/form submission.
In `@package.json`:
- Line 16: package.json's packageManager is set to "pnpm@11.2.2+..." but GitHub
Actions use pnpm/action-setup with with.version pinned to 9.x; update the
workflow steps that call pnpm/action-setup (the steps that include with.version)
to use "11.2.2" (or remove the explicit with.version to let the action pick the
packageManager field) so the CI pnpm version matches the packageManager field;
search for usages of pnpm/action-setup and the with.version key and replace the
version value accordingly.
---
Minor comments:
In `@apps/website/src/lib/components/card/card.stories.svelte`:
- Around line 23-26: Replace the inappropriate placeholder string used in the
Storybook docs for the Card component: update the docs.description.story value
(currently 'your mother') in card.stories.svelte to a professional, descriptive
sentence that summarizes the story purpose (e.g., describing what the Card
demonstrates, props showcased, and intended usage). Locate the docs block that
sets description.story and replace the text with concise, professional copy such
as "Demonstrates the Card component with title, image, and actions; use this
story to preview props and layout variants."
In `@apps/website/src/lib/components/drawer/drawer.svelte`:
- Around line 72-74: Remove the stray debug console logging in the Escape key
handler: inside the keydown handler where it checks if (e.key === 'Escape') and
calls dispatchCloseEvent(), delete the console.info('YO WTF', e) statement so
only dispatchCloseEvent() is executed; ensure no other debug logs remain in the
same handler or related functions in drawer.svelte.
In `@apps/website/src/routes/dashboard/`+page.svelte:
- Around line 155-177: The third summary card is passing the entire upcoming
array (billsByStatus.data.upcoming) instead of a numeric count and is labeled
“Due This Week” even though the remote only buckets as "upcoming"; update the
summary constant so the third item uses value:
billsByStatus.data.upcoming.length and either change label to "Upcoming" or
implement a week-scoped filter before assigning value (e.g., derive a
weekFilteredUpcoming array from billsByStatus.data.upcoming and use its .length)
to keep label and data semantically correct.
---
Nitpick comments:
In @.claude/settings.local.json:
- Around line 7-8: The current broad permissions "Bash(cat *)" and "Bash(ls *)"
should be narrowed to least-privilege scoped globs; replace those entries with
path-limited patterns that only allow required directories/files (e.g.,
"Bash(cat project/*)" or "Bash(cat src/**/*.js)" and "Bash(ls project/*)") or
explicit file lists for the specific tools/data the agent needs, updating the
entries in the .claude/settings.local.json array where "Bash(cat *)" and
"Bash(ls *)" appear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
jhechtf
force-pushed
the
chore/misc-improvements
branch
from
May 28, 2026 02:53
f13ac87 to
3225d0f
Compare
jhechtf
force-pushed
the
chore/misc-improvements
branch
from
May 28, 2026 16:45
3225d0f to
b3364f4
Compare
jhechtf
force-pushed
the
chore/misc-improvements
branch
from
May 28, 2026 16:45
b3364f4 to
81b750a
Compare
jhechtf
force-pushed
the
chore/misc-improvements
branch
from
May 31, 2026 02:04
5a87637 to
bc6a539
Compare
jhechtf
force-pushed
the
chore/misc-improvements
branch
from
May 31, 2026 02:10
bc6a539 to
fcd5811
Compare
jhechtf
force-pushed
the
chore/misc-improvements
branch
from
May 31, 2026 03:03
fcd5811 to
b15cf85
Compare
Contributor
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 (3)
apps/website/src/routes/dashboard/+page.svelte (3)
26-26:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTypo in URL:
/dashbaord/should be/dashboard/.This will cause the payment drawer to navigate to an invalid route.
🐛 Proposed fix
- let makeOrUpdatePayment = makeShowDrawerUtil('/dashbaord/payments/create'); + let makeOrUpdatePayment = makeShowDrawerUtil('/dashboard/payments/create');🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/`+page.svelte at line 26, The makeOrUpdatePayment initialization uses a misspelled route string; update the call to makeShowDrawerUtil in the assignment to makeOrUpdatePayment to use the correct path '/dashboard/payments/create' (fix '/dashbaord/' → '/dashboard/') so the payment drawer navigates to the valid route.
191-198:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win"Total Outstanding" is hardcoded to $0.00.
The outstanding amount could be computed from unpaid bills in
billsByStatus.data.overdueandbillsByStatus.data.upcoming. Is this intentional placeholder or should it display the actual sum?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/`+page.svelte around lines 191 - 198, The displayed "Total Outstanding" is hardcoded to (0) — replace it with a computed sum from the billing data: iterate billsByStatus.data.overdue and billsByStatus.data.upcoming, filter/ensure amounts are numeric, sum their amount fields (e.g., with Array.prototype.reduce) and render that total using toLocaleString with style:'currency' and currency:'USD' where the current hardcoded {(0).toLocaleString(...)} is used; ensure you handle missing billsByStatus or .data safely (fallback to 0) so the component (in +page.svelte) won't crash when the API data is absent.
391-403:⚠️ Potential issue | 🟠 Major | ⚡ Quick win"Pay Overdue" button doesn't filter out bills without payment records.
Unlike "Pay This Week" (line 382), this handler doesn't filter for non-null payments, which could produce
payments[]=undefinedin the URL.🐛 Proposed fix
onclick={() => { const ids = billsByStatus.data.overdue + .filter((bill) => bill.payment !== null) .map((bill) => `payments[]=${bill.payment?.id}`) .join('&'); makeMultiplePayments.url = `/dashboard/payments/create?${ids}`; makeMultiplePayments.show = true; }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/website/src/routes/dashboard/`+page.svelte around lines 391 - 403, The Pay Overdue button's onclick handler builds a query string from billsByStatus.data.overdue but doesn't filter out bills with no payment, which can emit payments[]=undefined; update the onclick for that button (the handler that sets makeMultiplePayments.url and makeMultiplePayments.show) to first filter billsByStatus.data.overdue for bill.payment != null (or bill.payment?.id truthy) before mapping to `payments[]=${bill.payment.id}`, mirroring the filtering used in the "Pay This Week" flow so the URL contains only valid payment IDs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/website/src/routes/dashboard/`+page.svelte:
- Around line 77-78: Remove the debug $inspect call that was left in the Svelte
page: locate the reactive debug statement "$inspect(billsByStatus)" in the
+page.svelte component and delete that line so development-only logging is not
committed; ensure no other $inspect runs remain and that the reactive variable
billsByStatus is untouched.
---
Outside diff comments:
In `@apps/website/src/routes/dashboard/`+page.svelte:
- Line 26: The makeOrUpdatePayment initialization uses a misspelled route
string; update the call to makeShowDrawerUtil in the assignment to
makeOrUpdatePayment to use the correct path '/dashboard/payments/create' (fix
'/dashbaord/' → '/dashboard/') so the payment drawer navigates to the valid
route.
- Around line 191-198: The displayed "Total Outstanding" is hardcoded to (0) —
replace it with a computed sum from the billing data: iterate
billsByStatus.data.overdue and billsByStatus.data.upcoming, filter/ensure
amounts are numeric, sum their amount fields (e.g., with Array.prototype.reduce)
and render that total using toLocaleString with style:'currency' and
currency:'USD' where the current hardcoded {(0).toLocaleString(...)} is used;
ensure you handle missing billsByStatus or .data safely (fallback to 0) so the
component (in +page.svelte) won't crash when the API data is absent.
- Around line 391-403: The Pay Overdue button's onclick handler builds a query
string from billsByStatus.data.overdue but doesn't filter out bills with no
payment, which can emit payments[]=undefined; update the onclick for that button
(the handler that sets makeMultiplePayments.url and makeMultiplePayments.show)
to first filter billsByStatus.data.overdue for bill.payment != null (or
bill.payment?.id truthy) before mapping to `payments[]=${bill.payment.id}`,
mirroring the filtering used in the "Pay This Week" flow so the URL contains
only valid payment IDs.
🪄 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: 1e565c6c-ab65-409a-9f02-4ceba2338c97
📒 Files selected for processing (3)
.github/workflows/pr.ymlapps/website/src/routes/dashboard/+page.svelteapps/website/src/routes/dashboard/bills/edit/[ids=ulids]/+page.svelte
💤 Files with no reviewable changes (2)
- .github/workflows/pr.yml
- apps/website/src/routes/dashboard/bills/edit/[ids=ulids]/+page.svelte
Comment on lines
77
to
+78
|
|
||
| let filteredBills = $derived.by(() => { | ||
| if (!billsWithStatus.isSuccess) | ||
| return [] as NonNullable<typeof billsWithStatus.data>; | ||
| if (filter === 'all') return billsWithStatus.data; | ||
| if (filter === 'overdue') { | ||
| return billsWithStatus.data.filter((bill) => bill.status === 'overdue'); | ||
| } | ||
| if (filter === 'paid') { | ||
| return billsWithStatus.data.filter((bill) => bill.status === 'paid'); | ||
| } | ||
| }); | ||
| $inspect(billsByStatus); |
Contributor
There was a problem hiding this comment.
Remove debug $inspect statement before merging.
The $inspect rune will log reactive state changes to the console in development, but should not be committed to the main branch.
🧹 Proposed fix
let filter: Filters = $state('all');
-
- $inspect(billsByStatus);📝 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 filteredBills = $derived.by(() => { | |
| if (!billsWithStatus.isSuccess) | |
| return [] as NonNullable<typeof billsWithStatus.data>; | |
| if (filter === 'all') return billsWithStatus.data; | |
| if (filter === 'overdue') { | |
| return billsWithStatus.data.filter((bill) => bill.status === 'overdue'); | |
| } | |
| if (filter === 'paid') { | |
| return billsWithStatus.data.filter((bill) => bill.status === 'paid'); | |
| } | |
| }); | |
| $inspect(billsByStatus); | |
| let filter: Filters = $state('all'); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/website/src/routes/dashboard/`+page.svelte around lines 77 - 78, Remove
the debug $inspect call that was left in the Svelte page: locate the reactive
debug statement "$inspect(billsByStatus)" in the +page.svelte component and
delete that line so development-only logging is not committed; ensure no other
$inspect runs remain and that the reactive variable billsByStatus is untouched.
jhechtf
force-pushed
the
chore/misc-improvements
branch
from
May 31, 2026 04:31
b15cf85 to
8cc5064
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/website/postcss.config.cjs`:
- Line 1: Prettier formatting errors in apps/website/postcss.config.cjs are
causing CI to fail; run the formatter (prettier --write
apps/website/postcss.config.cjs) to reformat the file, ensure the file header
comment ("// lightningcss 1.32.0 incorrectly rejects some ::file-selector-button
selectors") and surrounding whitespace match Prettier's output, then stage and
commit the updated file so the CI lint/prettier check passes.
- Around line 16-18: Update the comment example that shows the transformation of
::file-selector-button:disabled:hover to match the implemented logic (only
non-user-action pseudo-classes are moved before ::file-selector-button while
user-action pseudo-classes like :hover remain after). Replace the incorrect
example (::file-selector-button:disabled:hover →
:disabled:hover::file-selector-button) with the correct result
(::file-selector-button:disabled:hover → :disabled::file-selector-button:hover),
referencing the ::file-selector-button selector and the distinction between
non-user-action pseudo-classes (e.g., :disabled) and user-action pseudo-classes
(e.g., :hover) so the comment aligns with the code handling in the block that
reorders pseudo-classes.
🪄 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: f3ba8db3-14fc-41c9-901e-27f74a5acf87
📒 Files selected for processing (4)
.github/workflows/pr.ymlapps/website/postcss.config.cjsapps/website/src/routes/dashboard/+page.svelteapps/website/src/routes/dashboard/bills/edit/[ids=ulids]/+page.svelte
💤 Files with no reviewable changes (2)
- .github/workflows/pr.yml
- apps/website/src/routes/dashboard/bills/edit/[ids=ulids]/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/website/src/routes/dashboard/+page.svelte
| plugins: { | ||
| tailwindcss: {}, | ||
| autoprefixer: {}, | ||
| // lightningcss 1.32.0 incorrectly rejects some ::file-selector-button selectors |
Contributor
There was a problem hiding this comment.
Fix Prettier formatting to unblock CI.
The pipeline is failing because Prettier has detected code style issues in this file. Run prettier --write apps/website/postcss.config.cjs to fix the formatting.
🧰 Tools
🪛 GitHub Actions: Pr Checks / 1_formatter.txt
[warning] 1-1: Prettier --check reported code style issues in this file. Run 'prettier --write' to fix.
🪛 GitHub Actions: Pr Checks / formatter
[warning] 1-1: Prettier: code style issues found. Run 'prettier --write' to fix.
[error] 1-1: Step 'pnpm prettier:check' failed because 'prettier . --check' reported formatting issues (exit code 1).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/website/postcss.config.cjs` at line 1, Prettier formatting errors in
apps/website/postcss.config.cjs are causing CI to fail; run the formatter
(prettier --write apps/website/postcss.config.cjs) to reformat the file, ensure
the file header comment ("// lightningcss 1.32.0 incorrectly rejects some
::file-selector-button selectors") and surrounding whitespace match Prettier's
output, then stage and commit the updated file so the CI lint/prettier check
passes.
Comment on lines
+16
to
+18
| // Move any non-user-action pseudo-classes from after ::file-selector-button to before it. | ||
| // e.g. ::file-selector-button:disabled → :disabled::file-selector-button | ||
| // ::file-selector-button:disabled:hover → :disabled:hover::file-selector-button |
Contributor
There was a problem hiding this comment.
Correct the comment example.
The example on line 18 is inconsistent with the actual code behavior. Based on the logic in lines 23-25, user-action pseudo-classes (:hover) remain after ::file-selector-button, while only non-user-action pseudo-classes are moved before it.
The actual transformation for ::file-selector-button:disabled:hover would be:
- Input:
::file-selector-button:disabled:hover - Output:
:disabled::file-selector-button:hover(not:disabled:hover::file-selector-button)
📝 Proposed fix for the comment
// Move any non-user-action pseudo-classes from after ::file-selector-button to before it.
// e.g. ::file-selector-button:disabled → :disabled::file-selector-button
- // ::file-selector-button:disabled:hover → :disabled:hover::file-selector-button
+ // ::file-selector-button:disabled:hover → :disabled::file-selector-button:hover🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/website/postcss.config.cjs` around lines 16 - 18, Update the comment
example that shows the transformation of ::file-selector-button:disabled:hover
to match the implemented logic (only non-user-action pseudo-classes are moved
before ::file-selector-button while user-action pseudo-classes like :hover
remain after). Replace the incorrect example
(::file-selector-button:disabled:hover → :disabled:hover::file-selector-button)
with the correct result (::file-selector-button:disabled:hover →
:disabled::file-selector-button:hover), referencing the ::file-selector-button
selector and the distinction between non-user-action pseudo-classes (e.g.,
:disabled) and user-action pseudo-classes (e.g., :hover) so the comment aligns
with the code handling in the block that reorders pseudo-classes.
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
New Features
Refactor
Chores