Make the duplicate-code check stricter and remove the duplication it finds - #1795
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change set lowers the jscpd threshold and consolidates repeated logic into shared helpers across scripts, booking and API flows, database access, authentication, deployment, and UI templates. Related tests, translations, documentation comments, and configuration are updated. ChangesShared refactors and feature integrations
Estimated code review effort: 5 (Critical) | ~120 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5909c38b1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🤖 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 `@scripts/mutation/generate.ts`:
- Around line 155-160: Update unaryMutants and UNARY_MUTATIONS so every
supported unary operator has an explicit table entry, using an empty array for
intentionally mutation-free operators; remove the ?? [] fallback so missing
operators fail through the required lookup behavior.
In `@src/features/admin/bulk-email.ts`:
- Around line 19-22: Update sendBulkEmails to clear or mark the bulk email draft
consumed before invoking the non-idempotent external send. Ensure
settings.update.bulkEmailDraft("") occurs before recordContacts and logActivity,
while preserving the existing send behavior and avoiding draft reuse after later
failures.
In `@src/features/admin/database-reset.ts`:
- Around line 52-53: Update handleDemoResetGet and demoResetPage so the flash
value supplied by publicFormPage’s render callback is accepted and passed
through to the demo-reset page renderer. Preserve the existing access wrapper
and ensure the flashed validation error from errorRedirect("/demo/reset", error)
appears on the confirmation page.
In `@src/features/api/booking.ts`:
- Around line 65-70: Update the date validation in the booking flow to inspect
body.date with an explicit type check before converting or validating it. Reject
missing and non-string values through the existing bookingError.invalidDate
response, and only pass a validated string to availableDates.includes and the
returned date; remove the ?? defaulting.
In `@src/features/public/site-page.ts`:
- Around line 58-64: Update the exported publicSlugRoute higher-order function
to declare an explicit return type for the outer function, using the
repository’s established route-handler type or equivalent. Keep the inner
handler’s existing Response | Promise<Response> behavior unchanged and ensure
the public API no longer relies on return-type inference.
In `@src/shared/find-by-id.ts`:
- Around line 14-22: Annotate the exported findByIdThen function with the named
FindByIdThen<R> return type instead of relying on inferred nested generic
function types. Preserve the existing fetch, build, and null-handling behavior
while applying the type to findByIdThen’s own return signature.
In `@src/shared/try-step.ts`:
- Around line 12-15: Update the catch block in try-step to narrow the caught
value before accessing .message: use the Error message for actual Error
instances and a safe string representation for null, undefined, strings, and
other thrown values, while preserving the existing label, error, and ok:false
result shape.
In `@src/ui/templates/admin/guide/components.tsx`:
- Around line 120-124: Annotate the exported renderGuideSections constant with
the repository’s concrete section-renderer type, while preserving its existing
sectionsRenderer callback and rendering behavior. Use the established type
symbol from the surrounding guide rendering code rather than relying on inferred
typing.
In `@src/ui/templates/admin/listings/aggregates.tsx`:
- Around line 66-92: Update ListingRunningTotalsSection so the recalculate
action URL is only passed when isReadOnly() is false, matching
ListingAggregateMismatchRow’s existing gating. Preserve the notice content while
ensuring read-only mode omits actionHref and therefore hides the recalculate
link.
In `@src/ui/templates/admin/settings/payment.tsx`:
- Line 263: Add an English locale entry for the SumUp guide label in the
appropriate message catalog, then update the SumUp PaymentGuideLink usage in the
payment settings component to pass the translated value from t(...). Keep the
displayed text unchanged and follow the existing Stripe and Square translation
patterns.
In `@src/ui/templates/attendee-table.tsx`:
- Around line 293-296: Update the comment immediately before the buildAnswerMaps
call to document that computeVisibilityMap sets visMap.answers true only when
opts.questionData is defined, making the non-null assertion on opts.questionData
safe. Preserve the existing explanation of the empty-array behavior.
In `@src/ui/templates/components/settings-section.tsx`:
- Line 15: Update SettingsSectionDetails and the settingsSectionWith wrapper to
accept and forward id and enctype form attributes, preserving existing behavior
for callers that omit them. Ensure config-driven settings forms can use these
fields through the wrapper.
In `@test/shared/admin-api-example.test.ts`:
- Line 18: Replace the self-derived ADMIN_API_EXAMPLE_ADMIN_LISTING expected
value in the admin API example test with an independently authored AdminListing
fixture or separate schema assertion. Ensure the test no longer derives both the
actual and expected values through toAdminListing(API_EXAMPLE_LISTING), while
preserving the existing comparison behavior.
In `@test/ui/templates/admin/built-sites.test.ts`:
- Line 245: Update the assertion in the missing BUNNY_API_KEY test to expect the
exact host error text, preserving the “its secrets can't be read” wording and
prefix.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1243938b-1899-4f8e-b639-6e76c1e2d065
📒 Files selected for processing (207)
scripts/bench/cold-start/first-request-child.tsscripts/find-unused-src.tsscripts/line-counts.tsscripts/mutation/child-process.tsscripts/mutation/generate.tsscripts/mutation/ignore.tsscripts/mutation/isolation.tsscripts/mutation/operators.tsscripts/mutation/runner.tsscripts/profile-cold-boot.tsscripts/static-cdn.tsscripts/stripe-mock.tsscripts/stripe-mock/install.tsscripts/unit-tests-report.tssrc/features/admin/api-groups.tssrc/features/admin/api-keys.tssrc/features/admin/api.tssrc/features/admin/attendee-form-model.tssrc/features/admin/attendee-form-routes.tssrc/features/admin/attendee-page.tssrc/features/admin/attendee-refunds.tssrc/features/admin/attendees-edit.tssrc/features/admin/attendees-route-helpers.tssrc/features/admin/built-sites.tssrc/features/admin/bulk-email.tssrc/features/admin/catalog-transfer/import.tssrc/features/admin/catalog-transfer/schema.tssrc/features/admin/confirmation.tssrc/features/admin/content-form-fields.tssrc/features/admin/database-reset.tssrc/features/admin/entity-write-tab.tssrc/features/admin/find-group.tssrc/features/admin/images.tssrc/features/admin/ledger/entries.tssrc/features/admin/listing-page.tssrc/features/admin/listings-edit.tssrc/features/admin/load-listing.tssrc/features/admin/news-form.tssrc/features/admin/questions.tssrc/features/admin/scanner.tssrc/features/admin/servicing.tsxsrc/features/admin/servicing/page.tsxsrc/features/admin/sessions.tssrc/features/admin/settings-helpers.tssrc/features/admin/settings-sumup.tssrc/features/admin/settings-superuser.tssrc/features/admin/settings-wallets.tssrc/features/admin/site-content.tssrc/features/admin/site-pages-form.tssrc/features/admin/site.tssrc/features/admin/support.tssrc/features/admin/update.tssrc/features/admin/users.tssrc/features/api/booking.tssrc/features/api/folded-booking.tssrc/features/api/listings.tssrc/features/api/packages.tssrc/features/api/payment-processing/classify.tssrc/features/api/payment-processing/create.tssrc/features/api/public-listing.tssrc/features/api/webhooks.tssrc/features/auth.tssrc/features/join.tssrc/features/public/news.tssrc/features/public/site-page.tssrc/features/public/ticket-form.tssrc/features/public/ticket-submit.tssrc/features/public/ticket-submit/parse.tssrc/features/public/ticket-submit/paths.tssrc/features/public/ticket-submit/prepare.tssrc/features/tickets/token-utils.tssrc/locales/en/attendees.jsonsrc/shared/accounting/backfill.tssrc/shared/accounting/rows.tssrc/shared/admin-api-example.tssrc/shared/app-forms.tssrc/shared/booking.tssrc/shared/booking/fold-tree.tssrc/shared/booking/model.tssrc/shared/booking/package-cap.tssrc/shared/builder.tssrc/shared/crypto/encryption.tssrc/shared/crypto/keys.tssrc/shared/db/attempt-lockout.tssrc/shared/db/attendees/capacity.tssrc/shared/db/attendees/servicing.tssrc/shared/db/capacity.tssrc/shared/db/client.tssrc/shared/db/common-schema.tssrc/shared/db/groups.tssrc/shared/db/listing-parents.tssrc/shared/db/login-attempts.tssrc/shared/db/logistics-agents.tssrc/shared/db/migrations.tssrc/shared/db/migrations/schema-sync.tssrc/shared/db/modifier-resolve.tssrc/shared/db/modifiers.tssrc/shared/db/query.tssrc/shared/db/questions/attendee-answers/save.tssrc/shared/db/restore-legacy-columns.tssrc/shared/db/table.tssrc/shared/db/token-attempts.tssrc/shared/deno-deploy-api.tssrc/shared/email.tssrc/shared/env.tssrc/shared/find-by-id.tssrc/shared/forms.tsxsrc/shared/listings-actions.tssrc/shared/order/evaluate.tssrc/shared/payment-helpers.tssrc/shared/payments.tssrc/shared/provider-types.tssrc/shared/refund-ledger.tssrc/shared/response-steps.tssrc/shared/rest/crud-api.tssrc/shared/rest/resource.tssrc/shared/seen-before.tssrc/shared/site-secrets.tssrc/shared/site-update.tssrc/shared/square.tssrc/shared/stripe.tssrc/shared/sumup-provider.tssrc/shared/try-step.tssrc/shared/types.tssrc/ui/client/admin/payment-test-buttons.tssrc/ui/client/order.tssrc/ui/templates/admin/activityLog.tsxsrc/ui/templates/admin/admin-page.tsxsrc/ui/templates/admin/api-keys.tsxsrc/ui/templates/admin/attendee-ledger-panel.tsxsrc/ui/templates/admin/attendee-page.tsxsrc/ui/templates/admin/attendees.tsxsrc/ui/templates/admin/bulk-actions.tsxsrc/ui/templates/admin/catalog-transfer.tsxsrc/ui/templates/admin/dashboard.tsxsrc/ui/templates/admin/debug.tsxsrc/ui/templates/admin/delete-link.tsxsrc/ui/templates/admin/deliveries.tsxsrc/ui/templates/admin/expected-actual.tsxsrc/ui/templates/admin/groups/list.tsxsrc/ui/templates/admin/groups/overview.tsxsrc/ui/templates/admin/guide/components.tsxsrc/ui/templates/admin/guide/integrations.tsxsrc/ui/templates/admin/ledger/formatting.tsxsrc/ui/templates/admin/ledger/statement.tsxsrc/ui/templates/admin/listing-table.tsxsrc/ui/templates/admin/listings/aggregates.tsxsrc/ui/templates/admin/listings/attendees.tsxsrc/ui/templates/admin/listings/capacity-rows.tsxsrc/ui/templates/admin/listings/edit-panel.tsxsrc/ui/templates/admin/listings/ledger-section.tsxsrc/ui/templates/admin/logistics.tsxsrc/ui/templates/admin/logout.tsxsrc/ui/templates/admin/money-adjust-section.tsxsrc/ui/templates/admin/news.tsxsrc/ui/templates/admin/questions.tsxsrc/ui/templates/admin/seeds.tsxsrc/ui/templates/admin/servicing-events.tsxsrc/ui/templates/admin/sessions.tsxsrc/ui/templates/admin/settings-advanced.tsxsrc/ui/templates/admin/settings-nag-banner.tsxsrc/ui/templates/admin/settings.tsxsrc/ui/templates/admin/settings/page-shell.tsxsrc/ui/templates/admin/settings/payment.tsxsrc/ui/templates/admin/settings/schema-form.tsxsrc/ui/templates/admin/settings/wallet-settings.tsxsrc/ui/templates/admin/share-rows.tsxsrc/ui/templates/admin/site-pages.tsxsrc/ui/templates/admin/site.tsxsrc/ui/templates/attendee-table.tsxsrc/ui/templates/checkin.tsxsrc/ui/templates/components/aggregate-sections.tsxsrc/ui/templates/components/inline-form-button.tsxsrc/ui/templates/components/item-list.tsxsrc/ui/templates/components/labelled-row.tsxsrc/ui/templates/components/new-tab-link.tsxsrc/ui/templates/components/phone-links.tsxsrc/ui/templates/components/prose-article.tsxsrc/ui/templates/components/prose-heading.tsxsrc/ui/templates/components/prose-section.tsxsrc/ui/templates/components/question-controls.tsxsrc/ui/templates/components/raw-paragraph.tsxsrc/ui/templates/components/reorder.tsxsrc/ui/templates/components/save-form.tsxsrc/ui/templates/components/settings-section.tsxsrc/ui/templates/components/table-scroll.tsxsrc/ui/templates/components/text-fields.tsxsrc/ui/templates/payment.tsxsrc/ui/templates/public/content-page.tsxsrc/ui/templates/public/homepage.tsxsrc/ui/templates/public/news.tsxsrc/ui/templates/public/order-gallery.tsxsrc/ui/templates/public/reservations/child-block.tssrc/ui/templates/public/reservations/day-config.tssrc/ui/templates/public/reservations/questions.tsxsrc/ui/templates/public/shared.tsxsrc/ui/templates/public/site-page.tsxsrc/ui/templates/tickets.tsxtest/features/public/ticket-form.test.tstest/lib/booking-model-day-counts.test.tstest/lib/booking-model-eligibility.test.tstest/lib/server-built-sites-update.test.tstest/shared/admin-api-example.test.tstest/shared/booking/package-cap.test.tstest/shared/seen-before.test.tstest/ui/templates/admin/built-sites.test.tstest/ui/templates/admin/ledger/statement.test.ts
💤 Files with no reviewable changes (1)
- test/shared/booking/package-cap.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8a01bb076c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
stefan-burke
force-pushed
the
claude/jscpd-min-tokens-dedup-jrx2qj
branch
from
July 12, 2026 20:48
8a01bb0 to
b8f01ee
Compare
stefan-burke
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/shared/db/attendees/capacity.ts (1)
616-624: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument the invariant behind the non-null assertions on
loads.get(day)!/m.get(day)!.These assertions are safe because
perDayLoadsalways emits an entry for every day passed in, but a short comment stating that would make the invariant explicit for future readers. Based on learnings, this repo prefers non-null assertions with a documenting comment over defensive branches when the missing case is genuinely impossible.📝 Example comment
+ // `loads` is built from `perDayLoads(groupRows, days)`, which always has + // an entry for every day in `days` — safe to assert non-null here. result.set( id, new Map(days.map((day) => [day, max_attendees - base - loads.get(day)!])), );Also applies to: 690-699
🤖 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 `@src/shared/db/attendees/capacity.ts` around lines 616 - 624, Add a concise comment next to the non-null assertions on loads.get(day)! in the capacity calculation and m.get(day)! at the other affected location, documenting that perDayLoads always emits an entry for every requested day. Keep the existing non-null assertions and calculation behavior unchanged.Source: Learnings
src/features/admin/site-content.ts (1)
45-50: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
siteEntityPostre-implements the patternformPostwas just extracted for.
gatedPost(SITE_FORM)((_session, form) => h(form))(request)is exactlyformPost(SITE_FORM)(h)(request). SinceformPostalready exists in#routes/auth.ts(imported here), this call site should use it instead of duplicating the discard-session pattern inline — directly aligned with this PR's dedup goal.♻️ Proposed fix
export const siteEntityPost = <T>(load: (id: number) => Promise<T | null>) => (handler: (item: T, form: FormParams) => Promise<Response>) => gatedEntityRoute<FormParams>((request, h) => - gatedPost(SITE_FORM)((_session, form) => h(form))(request), + formPost(SITE_FORM)(h)(request), )(load, handler);🤖 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 `@src/features/admin/site-content.ts` around lines 45 - 50, Update siteEntityPost to replace the inline gatedPost(SITE_FORM) session-discarding wrapper with the imported formPost(SITE_FORM)(h) helper, preserving the existing request handling and gatedEntityRoute behavior.
🤖 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 `@src/features/admin/attendees-route-helpers.ts`:
- Around line 174-180: Introduce a named route-params type for the attendee ID
shape, following the existing AttendeeRouteParams and ListingRouteParams
pattern, and use it in both the createAuthedHandler generic and attendeeFormPost
return type. Keep the handler behavior unchanged.
In `@src/features/admin/confirmation.ts`:
- Around line 108-122: Add an explicit return type to the exported
createVerifiedFormRoute helper, matching the return-type annotation used by
createConfirmedHandlers. Preserve the existing generic parameters, verification
flow, and callback behavior while annotating the return value at the function
declaration.
In `@src/features/public/site-page.ts`:
- Around line 62-67: Update publicSlugRoute to use a route-agnostic or generic
typed handler that supports both “GET /page/:slug” and “GET /news/:slug”
registrations. Preserve the existing slug extraction and requirePublicSite
behavior while removing the hard-coded page route contract.
In `@src/shared/db/attempt-lockout.ts`:
- Around line 23-31: Update lockoutActive so expired-row cleanup only deletes
the record when its stored locked-until timestamp still equals the lockedUntil
value observed by this call. Replace the unconditional deleteByField call with
an atomic conditional deletion using the available database helper or query
mechanism, while preserving the existing return behavior.
- Around line 23-31: Update lockoutActive to treat only null or undefined
lockedUntil values as inactive; allow timestamp 0 to proceed through expiration
checking and deleteByField cleanup.
- Around line 15-18: Update the exported curried function clearAttemptsFor to
explicitly annotate the outer function’s return type as the async inner callback
type, while preserving its existing parameters and deleteByField behavior.
In `@src/shared/email.ts`:
- Around line 119-124: Update getActiveEmailConfig to replace the
nullish-coalescing fallback with the repository’s explicit undefined/null check
pattern, matching getHostEmailConfig while preserving the site-config-first
behavior and null result when neither configuration exists.
In `@src/shared/payment-helpers.ts`:
- Around line 694-708: Move the test-only SignedTestWebhook type and
signedTestWebhook helper out of src/shared/payment-helpers.ts into a test
utility module such as test/test-utils/signed-webhook.ts. Update all test
imports to use the relocated helper through the established test-utils alias,
and remove the production exports while preserving the existing signing and
payload behavior.
In `@src/shared/response-steps.ts`:
- Around line 14-17: Update the exported featureGate outer curried function to
declare an explicit named return type, matching the existing convention used by
timedRunner. Preserve the inner action signature and current isOn/blocked
behavior, and reuse an appropriate existing type if available rather than
introducing an anonymous union.
In `@src/shared/rest/resource.ts`:
- Around line 223-228: Update the generic constraint for ResourceConfig/Resource
so Row requires an id: number property, then change runAfterCommit to access
row.id directly and remove the unsafe unknown double cast. Preserve the existing
afterCommit invocation and null/config guards.
In `@src/ui/templates/admin/bulk-actions.tsx`:
- Around line 28-39: Update the BulkActionPage/adminDuplicateGroupPage rendering
flow to accept the submitted duplicate-group values and pass them into the
corresponding TextField/TextFields inputs when re-rendering after an error.
Preserve the existing default values for the initial render, while ensuring
failed POST responses retain the submitted name, find/replace, and date fields.
In `@src/ui/templates/admin/debug.tsx`:
- Around line 172-194: Replace the branch-based dispatch in
AvailabilityStateBadge and StorageBackendBadge with exhaustive Record lookups
keyed by their respective state and backend unions. Store each badge’s variant
and label in the records, then render the selected entry for the incoming value,
preserving all existing labels and variants; retain JSX.Element return types.
In `@src/ui/templates/admin/share-rows.tsx`:
- Line 2: Remove the duplicate local UnavailablePublicUrlRow definition in
groups/overview.tsx and import UnavailablePublicUrlRow alongside
PublicTicketLink from the shared share-rows module. Preserve all existing usages
and behavior.
---
Outside diff comments:
In `@src/features/admin/site-content.ts`:
- Around line 45-50: Update siteEntityPost to replace the inline
gatedPost(SITE_FORM) session-discarding wrapper with the imported
formPost(SITE_FORM)(h) helper, preserving the existing request handling and
gatedEntityRoute behavior.
In `@src/shared/db/attendees/capacity.ts`:
- Around line 616-624: Add a concise comment next to the non-null assertions on
loads.get(day)! in the capacity calculation and m.get(day)! at the other
affected location, documenting that perDayLoads always emits an entry for every
requested day. Keep the existing non-null assertions and calculation behavior
unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e9f36a12-1c88-4753-98fa-cb6a9ccd5f1b
📒 Files selected for processing (221)
.jscpd.jsonTODO.mdscripts/bench/cold-start/first-request-child.tsscripts/find-unused-src.tsscripts/line-counts.tsscripts/mutation/child-process.tsscripts/mutation/generate.tsscripts/mutation/ignore.tsscripts/mutation/isolation.tsscripts/mutation/operators.tsscripts/mutation/runner.tsscripts/profile-cold-boot.tsscripts/static-cdn.tsscripts/stripe-mock.tsscripts/stripe-mock/install.tsscripts/timed-run.tsscripts/unit-tests-report.tssrc/features/admin/api-groups.tssrc/features/admin/api-keys.tssrc/features/admin/api.tssrc/features/admin/attendee-form-model.tssrc/features/admin/attendee-form-routes.tssrc/features/admin/attendee-page.tssrc/features/admin/attendee-refunds.tssrc/features/admin/attendees-edit.tssrc/features/admin/attendees-route-helpers.tssrc/features/admin/backup.tssrc/features/admin/built-sites.tssrc/features/admin/bulk-email.tssrc/features/admin/catalog-transfer/import.tssrc/features/admin/catalog-transfer/schema.tssrc/features/admin/confirmation.tssrc/features/admin/content-form-fields.tssrc/features/admin/database-reset.tssrc/features/admin/entity-write-tab.tssrc/features/admin/find-group.tssrc/features/admin/images.tssrc/features/admin/ledger/entries.tssrc/features/admin/listing-page.tssrc/features/admin/listings-edit.tssrc/features/admin/load-listing.tssrc/features/admin/news-form.tssrc/features/admin/questions.tssrc/features/admin/scanner.tssrc/features/admin/servicing.tsxsrc/features/admin/servicing/page.tsxsrc/features/admin/sessions.tssrc/features/admin/settings-helpers.tssrc/features/admin/settings-logistics.tssrc/features/admin/settings-sumup.tssrc/features/admin/settings-superuser.tssrc/features/admin/settings-wallets.tssrc/features/admin/site-content.tssrc/features/admin/site-pages-form.tssrc/features/admin/site.tssrc/features/admin/support.tssrc/features/admin/update.tssrc/features/admin/users.tssrc/features/api/booking.tssrc/features/api/folded-booking.tssrc/features/api/listings.tssrc/features/api/packages.tssrc/features/api/payment-processing/classify.tssrc/features/api/payment-processing/create.tssrc/features/api/public-listing.tssrc/features/api/webhooks.tssrc/features/auth.tssrc/features/join.tssrc/features/public/news.tssrc/features/public/site-page.tssrc/features/public/ticket-form.tssrc/features/public/ticket-submit.tssrc/features/public/ticket-submit/parse.tssrc/features/public/ticket-submit/paths.tssrc/features/public/ticket-submit/prepare.tssrc/features/public/ticket-submit/pricing.tssrc/features/tickets/token-utils.tssrc/locales/en/attendees.jsonsrc/locales/en/settings.jsonsrc/shared/accounting/backfill.tssrc/shared/accounting/rows.tssrc/shared/admin-api-example.tssrc/shared/app-forms.tssrc/shared/booking.tssrc/shared/booking/fold-tree.tssrc/shared/booking/model.tssrc/shared/booking/package-cap.tssrc/shared/builder.tssrc/shared/crypto/encryption.tssrc/shared/crypto/keys.tssrc/shared/db/attempt-lockout.tssrc/shared/db/attendees/capacity.tssrc/shared/db/attendees/servicing.tssrc/shared/db/capacity.tssrc/shared/db/client.tssrc/shared/db/common-schema.tssrc/shared/db/groups.tssrc/shared/db/images.tssrc/shared/db/listing-parents.tssrc/shared/db/login-attempts.tssrc/shared/db/logistics-agents.tssrc/shared/db/migrations.tssrc/shared/db/migrations/schema-sync.tssrc/shared/db/modifier-resolve.tssrc/shared/db/modifiers.tssrc/shared/db/query.tssrc/shared/db/questions/attendee-answers/save.tssrc/shared/db/restore-legacy-columns.tssrc/shared/db/table.tssrc/shared/db/token-attempts.tssrc/shared/deno-deploy-api.tssrc/shared/email.tssrc/shared/env.tssrc/shared/find-by-id.tssrc/shared/forms.tsxsrc/shared/listings-actions.tssrc/shared/order/evaluate.tssrc/shared/payment-helpers.tssrc/shared/payments.tssrc/shared/provider-types.tssrc/shared/refund-ledger.tssrc/shared/response-steps.tssrc/shared/rest/crud-api.tssrc/shared/rest/resource.tssrc/shared/seen-before.tssrc/shared/site-secrets.tssrc/shared/site-update.tssrc/shared/square.tssrc/shared/stripe.tssrc/shared/sumup-provider.tssrc/shared/try-step.tssrc/shared/types.tssrc/ui/client/admin/payment-test-buttons.tssrc/ui/client/order.tssrc/ui/templates/admin/activityLog.tsxsrc/ui/templates/admin/admin-page.tsxsrc/ui/templates/admin/api-keys.tsxsrc/ui/templates/admin/attendee-ledger-panel.tsxsrc/ui/templates/admin/attendee-page.tsxsrc/ui/templates/admin/attendees.tsxsrc/ui/templates/admin/availability-checker.tsxsrc/ui/templates/admin/bulk-actions.tsxsrc/ui/templates/admin/catalog-transfer.tsxsrc/ui/templates/admin/dashboard.tsxsrc/ui/templates/admin/debug.tsxsrc/ui/templates/admin/delete-link.tsxsrc/ui/templates/admin/deliveries.tsxsrc/ui/templates/admin/detail-rows.tsxsrc/ui/templates/admin/expected-actual.tsxsrc/ui/templates/admin/groups/list.tsxsrc/ui/templates/admin/groups/overview.tsxsrc/ui/templates/admin/guide/components.tsxsrc/ui/templates/admin/guide/integrations.tsxsrc/ui/templates/admin/ledger/formatting.tsxsrc/ui/templates/admin/ledger/statement.tsxsrc/ui/templates/admin/listing-qr.tsxsrc/ui/templates/admin/listing-table.tsxsrc/ui/templates/admin/listings/aggregates.tsxsrc/ui/templates/admin/listings/attendees.tsxsrc/ui/templates/admin/listings/capacity-rows.tsxsrc/ui/templates/admin/listings/edit-panel.tsxsrc/ui/templates/admin/listings/ledger-section.tsxsrc/ui/templates/admin/logistics.tsxsrc/ui/templates/admin/logout.tsxsrc/ui/templates/admin/money-adjust-section.tsxsrc/ui/templates/admin/news.tsxsrc/ui/templates/admin/questions.tsxsrc/ui/templates/admin/seeds.tsxsrc/ui/templates/admin/servicing-events.tsxsrc/ui/templates/admin/sessions.tsxsrc/ui/templates/admin/settings-advanced.tsxsrc/ui/templates/admin/settings-nag-banner.tsxsrc/ui/templates/admin/settings.tsxsrc/ui/templates/admin/settings/page-shell.tsxsrc/ui/templates/admin/settings/payment.tsxsrc/ui/templates/admin/settings/schema-form.tsxsrc/ui/templates/admin/settings/wallet-settings.tsxsrc/ui/templates/admin/share-rows.tsxsrc/ui/templates/admin/site-pages.tsxsrc/ui/templates/admin/site.tsxsrc/ui/templates/attendee-table.tsxsrc/ui/templates/checkin.tsxsrc/ui/templates/components/aggregate-sections.tsxsrc/ui/templates/components/div-with-class.tsxsrc/ui/templates/components/inline-form-button.tsxsrc/ui/templates/components/item-list.tsxsrc/ui/templates/components/labelled-row.tsxsrc/ui/templates/components/new-tab-link.tsxsrc/ui/templates/components/phone-links.tsxsrc/ui/templates/components/prose-article.tsxsrc/ui/templates/components/prose-heading.tsxsrc/ui/templates/components/prose-section.tsxsrc/ui/templates/components/question-controls.tsxsrc/ui/templates/components/raw-paragraph.tsxsrc/ui/templates/components/reorder.tsxsrc/ui/templates/components/save-form.tsxsrc/ui/templates/components/settings-section.tsxsrc/ui/templates/components/table-scroll.tsxsrc/ui/templates/components/text-fields.tsxsrc/ui/templates/payment.tsxsrc/ui/templates/public/content-page.tsxsrc/ui/templates/public/homepage.tsxsrc/ui/templates/public/news.tsxsrc/ui/templates/public/order-gallery.tsxsrc/ui/templates/public/reservations/child-block.tssrc/ui/templates/public/reservations/day-config.tssrc/ui/templates/public/reservations/questions.tsxsrc/ui/templates/public/shared.tsxsrc/ui/templates/public/site-page.tsxsrc/ui/templates/tickets.tsxtest/features/public/ticket-form.test.tstest/lib/booking-model-day-counts.test.tstest/lib/booking-model-eligibility.test.tstest/lib/i18n-coverage.test.tstest/lib/server-built-sites-update.test.tstest/shared/admin-api-example.test.tstest/shared/booking/package-cap.test.tstest/shared/seen-before.test.tstest/ui/templates/admin/built-sites.test.tstest/ui/templates/admin/expected-actual.test.tsxtest/ui/templates/admin/ledger/statement.test.ts
💤 Files with no reviewable changes (1)
- test/shared/booking/package-cap.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8f01eedc0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
stefan-burke
force-pushed
the
claude/jscpd-min-tokens-dedup-jrx2qj
branch
from
July 12, 2026 21:44
b8f01ee to
c711218
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/shared/db/migrations/schema-sync.ts (1)
1-659: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffFile exceeds the ~400-line guideline.
schema-sync.tsnow spans well past 400 lines. Consider splitting the step definitions (applySchemaChanges,syncIndexes,syncTriggers,backfillListingAttendees/backfillListingAggregates,dropDeprecatedAttendeeColumns,verifyCurrentAppSchema) into focused modules now thatstepUsingdecouples each from its probe, since this refactor is a natural point to also address file size.As per coding guidelines, "Keep source and test files under approximately 400 lines; split overloaded files into focused modules and separate pure logic from I/O."
🤖 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 `@src/shared/db/migrations/schema-sync.ts` around lines 1 - 659, The schema-sync module exceeds the project’s size guideline and contains several separable migration concerns. Split the step definitions and related logic around applySchemaChanges, syncIndexes, syncTriggers, backfillListingAttendees, backfillListingAggregates, dropDeprecatedAttendeeColumns, and verifyCurrentAppSchema into focused modules, reusing stepUsing and shared helpers where needed; preserve their existing behavior and exports.Source: Coding guidelines
♻️ Duplicate comments (3)
src/shared/db/attempt-lockout.ts (1)
23-32: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftTOCTOU race in expired-lockout cleanup still unresolved.
lockoutActivestill unconditionally deletes the row byhashedIponce it decides the lockout is expired. A concurrent request could have already written a fresh lockout for the same IP between the read and this delete, and that fresh lockout would be silently discarded, losing rate-limit state. This was flagged on a prior commit and hasn't changed — delete should be conditioned on the stored value still matching what was read (e.g.DELETE ... WHERE ip = ? AND locked_until = ?), ideally in one atomic statement.🤖 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 `@src/shared/db/attempt-lockout.ts` around lines 23 - 32, The expired-lockout cleanup in lockoutActive must avoid deleting a newer lockout written after the read. Replace the unconditional deleteByField call with an atomic conditional deletion that matches both hashedIp and the previously read lockedUntil value, using the database helper or query mechanism that supports this predicate.src/shared/payment-helpers.ts (1)
694-709: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest-only helper still lives in production source.
SignedTestWebhook/signedTestWebhookremain exported fromsrc/shared/payment-helpers.tsdespite being documented purely for test use. This was flagged previously and still applies — consider moving it totest/test-utils/.🤖 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 `@src/shared/payment-helpers.ts` around lines 694 - 709, Move the test-only SignedTestWebhook type and signedTestWebhook helper out of src/shared/payment-helpers.ts into test/test-utils/. Update all test imports and usages to reference the new test utility, and remove their production exports while preserving the existing payload signing behavior.Source: Coding guidelines
src/ui/templates/admin/bulk-actions.tsx (1)
34-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDuplicate-group form still doesn't refill submitted values on error.
This mirrors the previously flagged, still-unresolved gap:
adminDuplicateGroupPageused to accept avaluesobject (per the line-range summary), but this revision drops it entirely in favor of the sharedBulkActionPagetype, sovalue={${group.name} (copy)}and theTextFieldsfield arrays are now hardcoded with no way to carry submitted input back in. A failed POST will reset the new-name/find/replace/date-shift inputs even thougherroris still threaded through correctly.To fix without breaking the shared
BulkActionPagecontract used byactivateConfirmPage, consider a dedicated params type for the duplicate form (or an optionalvaluesfield on a page-specific signature) rather than reusing the generic 4-arg type for this page.Based on learnings, "Admin form pages (including bulk/duplicate-group forms) should re-fill the user's submitted values back into the relevant TextField/TextFields inputs after a failed POST/validation error (flash/form-refill pattern)."
#!/bin/bash # Check whether the route handler ever forwards submitted values to the render function. rg -n "adminDuplicateGroupPage" -B3 -A15 src/features/admin/bulk-actions.tsAlso applies to: 174-256
🤖 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 `@src/ui/templates/admin/bulk-actions.tsx` around lines 34 - 39, The duplicate-group page signature currently cannot receive submitted form values, so failed submissions reset all fields. Update adminDuplicateGroupPage to use a dedicated parameters type or page-specific signature that includes values, then populate the duplicate-name, find/replace, and date-shift TextField/TextFields inputs from those values while preserving the shared BulkActionPage contract for other pages such as activateConfirmPage.Source: Learnings
🤖 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 `@src/features/admin/ledger/entries.ts`:
- Around line 152-171: Update loadedEntryPage to accept a lazy loader function
returning Promise<Loaded | null> instead of a pre-started promise, then invoke
it exactly once when loading the entry. Update all callers to pass loader
thunks, preserving the existing null handling, flash application, and rendering
behavior.
In `@src/features/admin/listings-edit.ts`:
- Line 117: Rename the .find() callback parameter in chosenTemplateOrPicker from
t to a non-conflicting name such as template, and update the callback’s property
access accordingly; preserve the existing template lookup behavior.
In `@src/features/public/ticket-submit/pricing.ts`:
- Line 19: Update the import in pricing.ts so AnswerInfo remains a type-only
import while extractContact is imported as a runtime value, allowing
ReturnType<typeof extractContact> to resolve correctly.
In `@src/shared/accounting/backfill.ts`:
- Around line 87-96: Update the raw SQL in nextPaidAttendeeIds and
stampFromExistingStatement to assign explicit descriptive singular aliases with
AS to listing_attendees and transfers, respectively. Qualify each table-column
reference with its corresponding alias while preserving the existing filtering,
ordering, and pagination behavior.
In `@src/shared/admin-api-example.ts`:
- Line 179: Update the `/api/packages/:slug/book` response example to use the
paid booking example payload instead of `API_BOOK_FREE_EXAMPLE_JSON`, or define
a package-specific paid response payload and reference it in the response
configuration.
In `@src/ui/templates/admin/listings/edit-panel.tsx`:
- Around line 180-191: Replace the hardcoded recalculate URL in the
ListingAggregateMismatchNotice actionHref with the shared adminPath helper,
using the listingRecalculate route and listing.id parameter. Preserve the
existing isReadOnly() gating and undefined behavior for read-only listings.
In `@src/ui/templates/components/aggregate-sections.tsx`:
- Around line 46-53: Update sectionsRenderer to use the curried map utility from
`#fp` instead of the native sections.map method, matching the functional style
used by the other consolidated render helpers while preserving the existing
render order and JSX fragment output.
In `@src/ui/templates/public/site-page.tsx`:
- Around line 9-19: Restore the missing NavNode type import alongside SitePage
in the imports used by sitePagePage, so the items declaration in the
seoContentPage callback type-checks without changing its behavior.
---
Outside diff comments:
In `@src/shared/db/migrations/schema-sync.ts`:
- Around line 1-659: The schema-sync module exceeds the project’s size guideline
and contains several separable migration concerns. Split the step definitions
and related logic around applySchemaChanges, syncIndexes, syncTriggers,
backfillListingAttendees, backfillListingAggregates,
dropDeprecatedAttendeeColumns, and verifyCurrentAppSchema into focused modules,
reusing stepUsing and shared helpers where needed; preserve their existing
behavior and exports.
---
Duplicate comments:
In `@src/shared/db/attempt-lockout.ts`:
- Around line 23-32: The expired-lockout cleanup in lockoutActive must avoid
deleting a newer lockout written after the read. Replace the unconditional
deleteByField call with an atomic conditional deletion that matches both
hashedIp and the previously read lockedUntil value, using the database helper or
query mechanism that supports this predicate.
In `@src/shared/payment-helpers.ts`:
- Around line 694-709: Move the test-only SignedTestWebhook type and
signedTestWebhook helper out of src/shared/payment-helpers.ts into
test/test-utils/. Update all test imports and usages to reference the new test
utility, and remove their production exports while preserving the existing
payload signing behavior.
In `@src/ui/templates/admin/bulk-actions.tsx`:
- Around line 34-39: The duplicate-group page signature currently cannot receive
submitted form values, so failed submissions reset all fields. Update
adminDuplicateGroupPage to use a dedicated parameters type or page-specific
signature that includes values, then populate the duplicate-name, find/replace,
and date-shift TextField/TextFields inputs from those values while preserving
the shared BulkActionPage contract for other pages such as activateConfirmPage.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 66f2eccb-a2b5-4f37-9f36-61b3faf650d2
📒 Files selected for processing (223)
.jscpd.jsonTODO.mdscripts/bench/cold-start/first-request-child.tsscripts/find-unused-src.tsscripts/line-counts.tsscripts/mutation/child-process.tsscripts/mutation/generate.tsscripts/mutation/ignore.tsscripts/mutation/isolation.tsscripts/mutation/operators.tsscripts/mutation/runner.tsscripts/profile-cold-boot.tsscripts/static-cdn.tsscripts/stripe-mock.tsscripts/stripe-mock/install.tsscripts/timed-run.tsscripts/unit-tests-report.tssrc/features/admin/api-groups.tssrc/features/admin/api-keys.tssrc/features/admin/api.tssrc/features/admin/attendee-form-model.tssrc/features/admin/attendee-form-routes.tssrc/features/admin/attendee-page.tssrc/features/admin/attendee-refunds.tssrc/features/admin/attendees-edit.tssrc/features/admin/attendees-route-helpers.tssrc/features/admin/backup.tssrc/features/admin/built-sites.tssrc/features/admin/bulk-email.tssrc/features/admin/catalog-transfer/import.tssrc/features/admin/catalog-transfer/schema.tssrc/features/admin/confirmation.tssrc/features/admin/content-form-fields.tssrc/features/admin/database-reset.tssrc/features/admin/entity-write-tab.tssrc/features/admin/find-group.tssrc/features/admin/images.tssrc/features/admin/ledger/entries.tssrc/features/admin/listing-page.tssrc/features/admin/listings-edit.tssrc/features/admin/load-listing.tssrc/features/admin/news-form.tssrc/features/admin/questions.tssrc/features/admin/scanner.tssrc/features/admin/servicing.tsxsrc/features/admin/servicing/page.tsxsrc/features/admin/sessions.tssrc/features/admin/settings-helpers.tssrc/features/admin/settings-logistics.tssrc/features/admin/settings-sumup.tssrc/features/admin/settings-superuser.tssrc/features/admin/settings-wallets.tssrc/features/admin/site-content.tssrc/features/admin/site-pages-form.tssrc/features/admin/site.tssrc/features/admin/support.tssrc/features/admin/update.tssrc/features/admin/users.tssrc/features/api/booking.tssrc/features/api/folded-booking.tssrc/features/api/listings.tssrc/features/api/packages.tssrc/features/api/payment-processing/classify.tssrc/features/api/payment-processing/create.tssrc/features/api/public-listing.tssrc/features/api/webhooks.tssrc/features/auth.tssrc/features/join.tssrc/features/public/news.tssrc/features/public/site-page.tssrc/features/public/ticket-form.tssrc/features/public/ticket-submit.tssrc/features/public/ticket-submit/parse.tssrc/features/public/ticket-submit/paths.tssrc/features/public/ticket-submit/prepare.tssrc/features/public/ticket-submit/pricing.tssrc/features/tickets/token-utils.tssrc/locales/en/attendees.jsonsrc/locales/en/settings.jsonsrc/shared/accounting/backfill.tssrc/shared/accounting/rows.tssrc/shared/admin-api-example.tssrc/shared/app-forms.tssrc/shared/booking.tssrc/shared/booking/fold-tree.tssrc/shared/booking/model.tssrc/shared/booking/package-cap.tssrc/shared/builder.tssrc/shared/crypto/encryption.tssrc/shared/crypto/keys.tssrc/shared/db/attempt-lockout.tssrc/shared/db/attendees/capacity.tssrc/shared/db/attendees/servicing.tssrc/shared/db/capacity.tssrc/shared/db/client.tssrc/shared/db/common-schema.tssrc/shared/db/groups.tssrc/shared/db/images.tssrc/shared/db/listing-parents.tssrc/shared/db/login-attempts.tssrc/shared/db/logistics-agents.tssrc/shared/db/migrations.tssrc/shared/db/migrations/schema-sync.tssrc/shared/db/modifier-resolve.tssrc/shared/db/modifiers.tssrc/shared/db/query.tssrc/shared/db/questions/attendee-answers/save.tssrc/shared/db/restore-legacy-columns.tssrc/shared/db/table.tssrc/shared/db/token-attempts.tssrc/shared/deno-deploy-api.tssrc/shared/email.tssrc/shared/env.tssrc/shared/find-by-id.tssrc/shared/forms.tsxsrc/shared/listings-actions.tssrc/shared/order/evaluate.tssrc/shared/payment-helpers.tssrc/shared/payments.tssrc/shared/provider-types.tssrc/shared/refund-ledger.tssrc/shared/response-steps.tssrc/shared/rest/crud-api.tssrc/shared/rest/resource.tssrc/shared/seen-before.tssrc/shared/site-secrets.tssrc/shared/site-update.tssrc/shared/square.tssrc/shared/stripe.tssrc/shared/sumup-provider.tssrc/shared/try-step.tssrc/shared/types.tssrc/ui/client/admin/payment-test-buttons.tssrc/ui/client/order.tssrc/ui/templates/admin/activityLog.tsxsrc/ui/templates/admin/admin-page.tsxsrc/ui/templates/admin/api-keys.tsxsrc/ui/templates/admin/attendee-ledger-panel.tsxsrc/ui/templates/admin/attendee-page.tsxsrc/ui/templates/admin/attendees.tsxsrc/ui/templates/admin/availability-checker.tsxsrc/ui/templates/admin/bulk-actions.tsxsrc/ui/templates/admin/catalog-transfer.tsxsrc/ui/templates/admin/dashboard.tsxsrc/ui/templates/admin/debug.tsxsrc/ui/templates/admin/delete-link.tsxsrc/ui/templates/admin/deliveries.tsxsrc/ui/templates/admin/detail-rows.tsxsrc/ui/templates/admin/expected-actual.tsxsrc/ui/templates/admin/groups/list.tsxsrc/ui/templates/admin/groups/overview.tsxsrc/ui/templates/admin/guide/components.tsxsrc/ui/templates/admin/guide/integrations.tsxsrc/ui/templates/admin/ledger/formatting.tsxsrc/ui/templates/admin/ledger/statement.tsxsrc/ui/templates/admin/listing-qr.tsxsrc/ui/templates/admin/listing-table.tsxsrc/ui/templates/admin/listings/aggregates.tsxsrc/ui/templates/admin/listings/attendees.tsxsrc/ui/templates/admin/listings/capacity-rows.tsxsrc/ui/templates/admin/listings/edit-panel.tsxsrc/ui/templates/admin/listings/ledger-section.tsxsrc/ui/templates/admin/logistics.tsxsrc/ui/templates/admin/logout.tsxsrc/ui/templates/admin/money-adjust-section.tsxsrc/ui/templates/admin/news.tsxsrc/ui/templates/admin/questions.tsxsrc/ui/templates/admin/seeds.tsxsrc/ui/templates/admin/servicing-events.tsxsrc/ui/templates/admin/sessions.tsxsrc/ui/templates/admin/settings-advanced.tsxsrc/ui/templates/admin/settings-nag-banner.tsxsrc/ui/templates/admin/settings.tsxsrc/ui/templates/admin/settings/page-shell.tsxsrc/ui/templates/admin/settings/payment.tsxsrc/ui/templates/admin/settings/schema-form.tsxsrc/ui/templates/admin/settings/wallet-settings.tsxsrc/ui/templates/admin/share-rows.tsxsrc/ui/templates/admin/site-pages.tsxsrc/ui/templates/admin/site.tsxsrc/ui/templates/attendee-table.tsxsrc/ui/templates/checkin.tsxsrc/ui/templates/components/aggregate-sections.tsxsrc/ui/templates/components/div-with-class.tsxsrc/ui/templates/components/inline-form-button.tsxsrc/ui/templates/components/item-list.tsxsrc/ui/templates/components/labelled-row.tsxsrc/ui/templates/components/new-tab-link.tsxsrc/ui/templates/components/phone-links.tsxsrc/ui/templates/components/prose-article.tsxsrc/ui/templates/components/prose-heading.tsxsrc/ui/templates/components/prose-section.tsxsrc/ui/templates/components/question-controls.tsxsrc/ui/templates/components/raw-paragraph.tsxsrc/ui/templates/components/reorder.tsxsrc/ui/templates/components/save-form.tsxsrc/ui/templates/components/settings-section.tsxsrc/ui/templates/components/table-scroll.tsxsrc/ui/templates/components/text-fields.tsxsrc/ui/templates/payment.tsxsrc/ui/templates/public/content-page.tsxsrc/ui/templates/public/homepage.tsxsrc/ui/templates/public/news.tsxsrc/ui/templates/public/order-gallery.tsxsrc/ui/templates/public/reservations/child-block.tssrc/ui/templates/public/reservations/day-config.tssrc/ui/templates/public/reservations/questions.tsxsrc/ui/templates/public/shared.tsxsrc/ui/templates/public/site-page.tsxsrc/ui/templates/tickets.tsxtest/features/public/ticket-form.test.tstest/lib/booking-model-day-counts.test.tstest/lib/booking-model-eligibility.test.tstest/lib/i18n-coverage.test.tstest/lib/server-built-sites-update.test.tstest/shared/admin-api-example.test.tstest/shared/booking/package-cap.test.tstest/shared/seen-before.test.tstest/shared/try-step.test.tstest/ui/templates/admin/built-sites.test.tstest/ui/templates/admin/expected-actual.test.tsxtest/ui/templates/admin/ledger/statement.test.tstest/ui/templates/admin/listings/edit-panel.test.ts
💤 Files with no reviewable changes (1)
- test/shared/booking/package-cap.test.ts
stefan-burke
force-pushed
the
claude/jscpd-min-tokens-dedup-jrx2qj
branch
from
July 13, 2026 03:56
c711218 to
2eae1ae
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/templates/admin/settings/payment.tsx (1)
289-291: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winSumUpForm still has hard-coded copy instead of
t().The
where_to_findlabel was fixed to uset(...), but the hint paragraph (Lines 289-291) andtestLabel/updateLabel(Lines 302-303) remain literal strings, unlike the equivalentStripeFormfields which all go throught("settings.stripe.*").♻️ Proposed fix
- {s.sumupKeyConfigured - ? "A SumUp API key is currently configured. Enter new credentials below to replace them." - : "No SumUp API key is configured. Enter your SumUp credentials to enable SumUp payments."} + {s.sumupKeyConfigured + ? t("settings.sumup.configured_hint") + : t("settings.sumup.not_configured_hint")}- testLabel="Test Connection" - updateLabel="Update SumUp Credentials" + testLabel={t("settings.sumup.test_connection")} + updateLabel={t("settings.sumup.update_key")}As per coding guidelines,
src/**/*.{ts,tsx}files must "Keep user-facing copy insrc/locales/en/*.jsonand access it throught("key"); do not hard-code user-facing strings in templates."Also applies to: 302-303
🤖 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 `@src/ui/templates/admin/settings/payment.tsx` around lines 289 - 291, Update SumUpForm’s hint paragraph and testLabel/updateLabel props to use the existing translation function t() with settings.sumup translation keys, matching the equivalent StripeForm fields. Add any missing user-facing strings to the English locale file and preserve the current text and behavior through translations.Source: Coding guidelines
♻️ Duplicate comments (2)
src/ui/templates/public/site-page.tsx (1)
9-19: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winMissing
NavNodeimport — will fail type-checking.
NavNodeis used at Line 19 but never imported; onlySitePageis imported from#shared/types.ts. This producesTS2304: Cannot find name 'NavNode'. This was already flagged in a prior review pass but does not appear to have been addressed in this version of the file.🐛 Proposed fix
-import type { SitePage } from "`#shared/types.ts`"; +import type { NavNode, SitePage } from "`#shared/types.ts`";🤖 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 `@src/ui/templates/public/site-page.tsx` around lines 9 - 19, Import the NavNode type in the sitePagePage module alongside SitePage from `#shared/types.ts` so the items declaration resolves during type-checking.src/features/public/ticket-submit/pricing.ts (1)
19-19: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
extractContactcan't stay a type-only import.
ReturnType<typeof extractContact>at lines 42 and 82 requiresextractContactas a value binding, butimport type { AnswerInfo, extractContact }erases it entirely at compile time — this raises TS1361 ('extractContact' cannot be used as a value because it was imported using 'import type'). Split the import: keepAnswerInfotype-only, importextractContactas a value.This was already flagged on a previous commit and appears unresolved in the current code.
🐛 Proposed fix
-import type { AnswerInfo, extractContact } from "../ticket-form.ts"; +import type { AnswerInfo } from "../ticket-form.ts"; +import { extractContact } from "../ticket-form.ts";🤖 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 `@src/features/public/ticket-submit/pricing.ts` at line 19, Update the import in pricing.ts so AnswerInfo remains a type-only import while extractContact is imported as a runtime value, allowing the existing ReturnType<typeof extractContact> usages to compile.
🤖 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 `@src/features/admin/update.ts`:
- Around line 89-95: Update the deployAndReport call in the update flow so
successPrefix uses the localized t(...) lookup instead of the hard-coded
“Updated” string. Add or reuse an appropriate locale key, while leaving
logPrefix unchanged as the internal activity-log label.
In `@src/features/public/ticket-form.ts`:
- Around line 104-114: Define a named exported return type for resolvePageDate,
capturing both the successful date result and the invalid-date error result,
then replace the function’s inline union annotation with that type. Keep the
existing behavior and discriminating ok fields unchanged.
In `@src/features/public/ticket-submit.ts`:
- Line 361: Replace the inline flattening of ctx.childrenByParentId.values() in
the surrounding ticket submission logic with the existing allChildListings
helper, including the children assignment, so all child-listing aggregation uses
the shared implementation.
In `@src/shared/booking/package-cap.ts`:
- Around line 325-332: Add a concise comment immediately before the non-null
assertion in sharedChildGroupLimits, documenting that application invariants
guarantee every groupId from needs is present in ctx.groupRemainingByGroupId.
Keep the existing lookup and assertion unchanged.
In `@src/shared/site-update.ts`:
- Around line 51-75: Update deployAndReport to obtain the success flash message
through the existing localization helper t, using a new admin.update.deployed
key in the appropriate English locale JSON with the required sentence-case
wording. Pass the localized message the existing successPrefix and
result.value.name parameters require, while preserving the current response
flow.
In `@src/ui/templates/admin/api-keys.tsx`:
- Around line 293-308: Replace the hardcoded JSX intro in the admin entry of
endpointDocsSections with t("api_keys.admin_api_note"), and add that key with
the equivalent sentence-case text to the appropriate English locale JSON file.
Keep the public API intro and endpoint configuration unchanged.
In `@src/ui/templates/admin/delete-link.tsx`:
- Around line 11-13: Update the exported rowDeleteLink helper to explicitly
declare the return type of its outer curried function, while preserving the
existing inner callback parameter and JSX.Element return annotations.
In `@src/ui/templates/admin/servicing-events.tsx`:
- Around line 29-45: Update upcomingServicingRow to replace the inline
listing-count pluralization with the existing t() localization helper, using the
pluralized admin.servicing.listing_count key. Add the corresponding
singular/plural entry to admin.json following the format of other admin count
strings, while preserving the current listingCount value and details layout.
---
Outside diff comments:
In `@src/ui/templates/admin/settings/payment.tsx`:
- Around line 289-291: Update SumUpForm’s hint paragraph and
testLabel/updateLabel props to use the existing translation function t() with
settings.sumup translation keys, matching the equivalent StripeForm fields. Add
any missing user-facing strings to the English locale file and preserve the
current text and behavior through translations.
---
Duplicate comments:
In `@src/features/public/ticket-submit/pricing.ts`:
- Line 19: Update the import in pricing.ts so AnswerInfo remains a type-only
import while extractContact is imported as a runtime value, allowing the
existing ReturnType<typeof extractContact> usages to compile.
In `@src/ui/templates/public/site-page.tsx`:
- Around line 9-19: Import the NavNode type in the sitePagePage module alongside
SitePage from `#shared/types.ts` so the items declaration resolves during
type-checking.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 631582ad-ef9f-4495-831c-e94d4b453021
📒 Files selected for processing (224)
.jscpd.jsonTODO.mdscripts/bench/cold-start/first-request-child.tsscripts/find-unused-src.tsscripts/line-counts.tsscripts/mutation/child-process.tsscripts/mutation/generate.tsscripts/mutation/ignore.tsscripts/mutation/isolation.tsscripts/mutation/operators.tsscripts/mutation/runner.tsscripts/profile-cold-boot.tsscripts/static-cdn.tsscripts/stripe-mock.tsscripts/stripe-mock/install.tsscripts/timed-run.tsscripts/unit-tests-report.tssrc/features/admin/api-groups.tssrc/features/admin/api-keys.tssrc/features/admin/api.tssrc/features/admin/attendee-form-model.tssrc/features/admin/attendee-form-routes.tssrc/features/admin/attendee-page.tssrc/features/admin/attendee-refunds.tssrc/features/admin/attendees-edit.tssrc/features/admin/attendees-route-helpers.tssrc/features/admin/backup.tssrc/features/admin/built-sites.tssrc/features/admin/bulk-email.tssrc/features/admin/catalog-transfer/import.tssrc/features/admin/catalog-transfer/schema.tssrc/features/admin/confirmation.tssrc/features/admin/content-form-fields.tssrc/features/admin/database-reset.tssrc/features/admin/entity-write-tab.tssrc/features/admin/find-group.tssrc/features/admin/images.tssrc/features/admin/ledger/entries.tssrc/features/admin/listing-page.tssrc/features/admin/listings-edit.tssrc/features/admin/load-listing.tssrc/features/admin/news-form.tssrc/features/admin/questions.tssrc/features/admin/scanner.tssrc/features/admin/servicing.tsxsrc/features/admin/servicing/page.tsxsrc/features/admin/sessions.tssrc/features/admin/settings-helpers.tssrc/features/admin/settings-logistics.tssrc/features/admin/settings-sumup.tssrc/features/admin/settings-superuser.tssrc/features/admin/settings-wallets.tssrc/features/admin/site-content.tssrc/features/admin/site-pages-form.tssrc/features/admin/site.tssrc/features/admin/support.tssrc/features/admin/update.tssrc/features/admin/users.tssrc/features/api/booking.tssrc/features/api/folded-booking.tssrc/features/api/listings.tssrc/features/api/packages.tssrc/features/api/payment-processing/classify.tssrc/features/api/payment-processing/create.tssrc/features/api/public-listing.tssrc/features/api/webhooks.tssrc/features/auth.tssrc/features/join.tssrc/features/public/news.tssrc/features/public/site-page.tssrc/features/public/ticket-form.tssrc/features/public/ticket-payment.tssrc/features/public/ticket-submit.tssrc/features/public/ticket-submit/parse.tssrc/features/public/ticket-submit/paths.tssrc/features/public/ticket-submit/prepare.tssrc/features/public/ticket-submit/pricing.tssrc/features/tickets/token-utils.tssrc/locales/en/attendees.jsonsrc/locales/en/settings.jsonsrc/shared/accounting/backfill.tssrc/shared/accounting/rows.tssrc/shared/admin-api-example.tssrc/shared/app-forms.tssrc/shared/booking.tssrc/shared/booking/fold-tree.tssrc/shared/booking/model.tssrc/shared/booking/package-cap.tssrc/shared/builder.tssrc/shared/crypto/encryption.tssrc/shared/crypto/keys.tssrc/shared/db/attempt-lockout.tssrc/shared/db/attendees/capacity.tssrc/shared/db/attendees/servicing.tssrc/shared/db/capacity.tssrc/shared/db/client.tssrc/shared/db/common-schema.tssrc/shared/db/groups.tssrc/shared/db/images.tssrc/shared/db/listing-parents.tssrc/shared/db/login-attempts.tssrc/shared/db/logistics-agents.tssrc/shared/db/migrations.tssrc/shared/db/migrations/schema-sync.tssrc/shared/db/modifier-resolve.tssrc/shared/db/modifiers.tssrc/shared/db/query.tssrc/shared/db/questions/attendee-answers/save.tssrc/shared/db/restore-legacy-columns.tssrc/shared/db/table.tssrc/shared/db/token-attempts.tssrc/shared/deno-deploy-api.tssrc/shared/email.tssrc/shared/env.tssrc/shared/find-by-id.tssrc/shared/forms.tsxsrc/shared/listings-actions.tssrc/shared/order/evaluate.tssrc/shared/payment-helpers.tssrc/shared/payments.tssrc/shared/provider-types.tssrc/shared/refund-ledger.tssrc/shared/response-steps.tssrc/shared/rest/crud-api.tssrc/shared/rest/resource.tssrc/shared/seen-before.tssrc/shared/site-secrets.tssrc/shared/site-update.tssrc/shared/square.tssrc/shared/stripe.tssrc/shared/sumup-provider.tssrc/shared/try-step.tssrc/shared/types.tssrc/ui/client/admin/payment-test-buttons.tssrc/ui/client/order.tssrc/ui/templates/admin/activityLog.tsxsrc/ui/templates/admin/admin-page.tsxsrc/ui/templates/admin/api-keys.tsxsrc/ui/templates/admin/attendee-ledger-panel.tsxsrc/ui/templates/admin/attendee-page.tsxsrc/ui/templates/admin/attendees.tsxsrc/ui/templates/admin/availability-checker.tsxsrc/ui/templates/admin/bulk-actions.tsxsrc/ui/templates/admin/catalog-transfer.tsxsrc/ui/templates/admin/dashboard.tsxsrc/ui/templates/admin/debug.tsxsrc/ui/templates/admin/delete-link.tsxsrc/ui/templates/admin/deliveries.tsxsrc/ui/templates/admin/detail-rows.tsxsrc/ui/templates/admin/expected-actual.tsxsrc/ui/templates/admin/groups/list.tsxsrc/ui/templates/admin/groups/overview.tsxsrc/ui/templates/admin/guide/components.tsxsrc/ui/templates/admin/guide/integrations.tsxsrc/ui/templates/admin/ledger/formatting.tsxsrc/ui/templates/admin/ledger/statement.tsxsrc/ui/templates/admin/listing-qr.tsxsrc/ui/templates/admin/listing-table.tsxsrc/ui/templates/admin/listings/aggregates.tsxsrc/ui/templates/admin/listings/attendees.tsxsrc/ui/templates/admin/listings/capacity-rows.tsxsrc/ui/templates/admin/listings/edit-panel.tsxsrc/ui/templates/admin/listings/ledger-section.tsxsrc/ui/templates/admin/logistics.tsxsrc/ui/templates/admin/logout.tsxsrc/ui/templates/admin/money-adjust-section.tsxsrc/ui/templates/admin/news.tsxsrc/ui/templates/admin/questions.tsxsrc/ui/templates/admin/seeds.tsxsrc/ui/templates/admin/servicing-events.tsxsrc/ui/templates/admin/sessions.tsxsrc/ui/templates/admin/settings-advanced.tsxsrc/ui/templates/admin/settings-nag-banner.tsxsrc/ui/templates/admin/settings.tsxsrc/ui/templates/admin/settings/page-shell.tsxsrc/ui/templates/admin/settings/payment.tsxsrc/ui/templates/admin/settings/schema-form.tsxsrc/ui/templates/admin/settings/wallet-settings.tsxsrc/ui/templates/admin/share-rows.tsxsrc/ui/templates/admin/site-pages.tsxsrc/ui/templates/admin/site.tsxsrc/ui/templates/attendee-table.tsxsrc/ui/templates/checkin.tsxsrc/ui/templates/components/aggregate-sections.tsxsrc/ui/templates/components/div-with-class.tsxsrc/ui/templates/components/inline-form-button.tsxsrc/ui/templates/components/item-list.tsxsrc/ui/templates/components/labelled-row.tsxsrc/ui/templates/components/new-tab-link.tsxsrc/ui/templates/components/phone-links.tsxsrc/ui/templates/components/prose-article.tsxsrc/ui/templates/components/prose-heading.tsxsrc/ui/templates/components/prose-section.tsxsrc/ui/templates/components/question-controls.tsxsrc/ui/templates/components/raw-paragraph.tsxsrc/ui/templates/components/reorder.tsxsrc/ui/templates/components/save-form.tsxsrc/ui/templates/components/settings-section.tsxsrc/ui/templates/components/table-scroll.tsxsrc/ui/templates/components/text-fields.tsxsrc/ui/templates/payment.tsxsrc/ui/templates/public/content-page.tsxsrc/ui/templates/public/homepage.tsxsrc/ui/templates/public/news.tsxsrc/ui/templates/public/order-gallery.tsxsrc/ui/templates/public/reservations/child-block.tssrc/ui/templates/public/reservations/day-config.tssrc/ui/templates/public/reservations/questions.tsxsrc/ui/templates/public/shared.tsxsrc/ui/templates/public/site-page.tsxsrc/ui/templates/tickets.tsxtest/features/public/ticket-form.test.tstest/lib/booking-model-day-counts.test.tstest/lib/booking-model-eligibility.test.tstest/lib/i18n-coverage.test.tstest/lib/server-built-sites-update.test.tstest/shared/admin-api-example.test.tstest/shared/booking/package-cap.test.tstest/shared/seen-before.test.tstest/shared/try-step.test.tstest/ui/templates/admin/built-sites.test.tstest/ui/templates/admin/expected-actual.test.tsxtest/ui/templates/admin/ledger/statement.test.tstest/ui/templates/admin/listings/edit-panel.test.ts
💤 Files with no reviewable changes (1)
- test/shared/booking/package-cap.test.ts
stefan-burke
enabled auto-merge
July 13, 2026 04:37
stefan-burke
force-pushed
the
claude/jscpd-min-tokens-dedup-jrx2qj
branch
from
July 13, 2026 05:19
ae87bdf to
a51b88f
Compare
Lowers the jscpd minimum-match threshold from 21 to 20 tokens and removes every duplicate the stricter setting surfaces (159 clones across the source tree). As with the previous notches, each clone was fixed by making the two copies one thing — a shared helper or a curried factory — never by reshaping code so the matcher stops matching. Highlights of the merges: - Booking: one child-eligibility check, one day-count path, one folded-booking finish, one availability check shared by the API and web pages. - Database access: id→name/column reads and "id set from rows" fold into the shared query helpers; hand-rolled group loops use the standard groupBy; a shared rate-limit "attempt lockout" mechanism replaces two parallel ones. - Admin routing: route guards fold onto the existing authed-form factories; new shared feature-gate, find-by-id, response-step and try-step helpers. - Payments: one checkout retrieve/sign path per concept; a shared new-tab-link component; the session-metadata type reuses the contact type. - UI: a listing-table module extracted from the dashboard, plus shared prose, table-scroll, labelled-row, settings-section and question-control components; the debug page is now a data table rendered by one fold. - News and site pages unify onto one content-form, one public slug route, and one SEO content page; expected-vs-actual recalculation shares one mechanism. - Scripts: a shared timed-runner and env builder; the real sleep/delay is imported instead of re-implemented. Also folds in review follow-ups: replaces `??`-defaulting with explicit checks in the booking date and unary-mutation paths, narrows non-Error throws in try-step, gives the read-only listing view no dead recalculate link, moves the SumUp guide label into the message catalog, and annotates exported helpers with explicit return types. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTrbBujCcBu8CUC56dy4tQ
stefan-burke
force-pushed
the
claude/jscpd-min-tokens-dedup-jrx2qj
branch
from
July 13, 2026 05:27
a51b88f to
a032024
Compare
Fold the last inlined "flatten every parent's children" spot in the ticket handler into the shared allChildListings helper, merge the three payment provider settings intros (Stripe, Square, SumUp) into one ProviderIntro block, and move the SumUp and service-event-count copy into the message catalog. Also tightens a few types and adds a missing-lookup invariant note raised in review; pre-existing copy and behaviour points are recorded in TODO.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DTrbBujCcBu8CUC56dy4tQ
stefan-burke
force-pushed
the
claude/jscpd-min-tokens-dedup-jrx2qj
branch
from
July 13, 2026 05:31
a032024 to
edfbd49
Compare
This was referenced Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this does
The project has an automatic check that fails the build if the same code appears in two places. This makes that check a little stricter — it now notices repeated blocks one step smaller than before (20 tokens instead of 21).
Turning the check up surfaced 159 spots where the same code lived in two places. This PR removes every one of them, so the build passes at the new, stricter setting.
How the duplication was removed
The rule is: two copies of the same code become one shared piece that both places use. Never a trick to make the checker look the other way while leaving the two copies in place. Some of the bigger consolidations:
Review follow-ups included
Several points raised in review are also fixed here:
A handful of other points raised in review are real but were already true before this change (not caused by it) — for example some update-page and API-docs wording that still lives in the code rather than the shared text file. Those are written up in
TODO.mdfor a focused follow-up rather than changed here, so this stays a duplicate-removal change.🤖 Generated with Claude Code
https://claude.ai/code/session_01DTrbBujCcBu8CUC56dy4tQ