fix: Apply the dev-vs-main review findings#114
Merged
Conversation
Changes: - Add `versions: [">=2.3.0"]` to the springdoc ignore entry - Note in the comment why the bound exists A bare `dependency-name` ignore suppresses Dependabot security updates as well as version updates. SecurityConfig serves /api-docs/**, /v3/api-docs/** and /swagger-ui/** with permitAll(), so an unauthenticated Swagger surface frozen at springdoc 2.2.0 would never receive a security PR. Bounding the ignore at >=2.3.0 keeps the Boot-4-only line blocked while letting 2.2.x patches and security fixes through. Notes: - Review finding 1 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Inert until this reaches main; Dependabot reads the config from the default branch only.
Changes: - Add a private nowUtc() helper to UserService and use it for all six timestamp writes - Stamp createdAt via LocalDateTime.now(ZoneOffset.UTC) in User.onCreate latestOf compares app_user.last_active_at against MAX(session.updated_at), which drizzle always writes in UTC. Stamping with LocalDateTime.now() used the JVM default zone, so the two operands only shared a clock base while the JVM happened to run UTC. Off Docker (local dev on Europe/Zagreb) the stamped column sat 1-2h ahead and always won the merge, and the same column held a different clock base per environment. Notes: - Review finding 6 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Prerequisite for finding 4: serialising these as UTC on the wire is only correct once the stored values genuinely are UTC. - Scoped to the user domain. Other domains still stamp via BaseEntity in the JVM zone, which is harmless today since nothing compares them to better-auth.
Changes: - Change UserDto.lastLoginAt / lastActiveAt and both AuthIdentity components to Instant - Convert the app_user column with toUtcInstant() in convertToUserDto - Replace toLocalDateTime with toInstant, handling Timestamp, Instant, OffsetDateTime and LocalDateTime, and logging an unmapped type instead of returning null Jackson serialized LocalDateTime without an offset, so the browser parsed "2026-07-24T13:05:11.123" as local time. In Zagreb that made a just-signed-in user render as "prije 2 sata", and shifted the dashboard's WAU/MAU window by the viewer's UTC offset so window-edge users landed in the wrong bucket and counts differed per admin. The wire format is now "...Z". The zod schemas stay z.string(), so parsing is unaffected; only the admin dashboard reads these two fields. Notes: - Review findings 4 and 10 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - toInstant converts a Timestamp via toLocalDateTime() first on purpose: the driver reads a zone-less column in the JVM zone, so calling toInstant() directly would re-apply that offset and shift the value again. - Depends on the preceding UTC-stamping commit; converting with ZoneOffset.UTC is only correct because the column is now written in UTC.
Changes:
- Add a <lombok.version> property set to 1.18.46
- Drop the hard-coded version from the lombok dependency
- Reference ${lombok.version} from the annotation processor path
The version was hard-coded in two places, so the last bump had to edit both
and a future bump that missed one would compile against a different lombok
than it declares.
Notes:
- Review finding 28 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md)
- The property name is the one spring-boot-dependencies uses, so declaring it
overrides the parent-managed version. Simply deleting both version elements
would have downgraded lombok to whatever Boot 3.1.0 pins, undoing 3c40b6a.
Changes: - Add AuthIdentityDao holding findByUserIds, deleteById and the timestamp coercion - Inject it into UserService and drop the direct EntityManager dependency - Remove the now-unused Timestamp, HashMap, OffsetDateTime and persistence imports Reading and deleting rows in better-auth's `user` / `session` tables is a different concern from the app_user profile service: that schema is owned by the frontend's auth library and has no JPA entities, so it needs native queries. Keeping it inline left UserService at 316 lines with two raw SQL statements sitting next to the profile logic. Notes: - Review finding 34 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - UserService is now 291 lines, still above the AGENTS.md 50-100 target. The remaining bulk is genuine profile logic and would need a separate split.
❌ Deploy Preview for disscount failed. Why did it fail? →
|
Contributor
|
Warning Review limit reached
Next review available in: 32 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (39)
WalkthroughChangesBackend identity and UTC timestamps
Dependency configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant UserService
participant AuthIdentityDao
participant EntityManager
UserService->>AuthIdentityDao: findByUserIds(userIds)
AuthIdentityDao->>EntityManager: execute native identity/session query
EntityManager-->>AuthIdentityDao: identity rows and timestamps
AuthIdentityDao-->>UserService: AuthIdentity map
UserService->>AuthIdentityDao: deleteById(userId)
AuthIdentityDao->>EntityManager: execute native user delete
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Changes: - Export MILLISECONDS_PER_DAY from utils/date.ts - Import it in user-activity.ts instead of redeclaring it AGENTS.md requires checking utils/ for an existing helper before writing a new one. The constant already existed but was module-private, so the dashboard util declared a second copy of the same value. Notes: - Review finding 31 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md)
…ending Changes: - Read isLoading and isError from useGetAllUsers in AdminUsersStats - Pass both into AdminActivityCard and render a Skeleton or the error text - Show the count only once the query has resolved successfully countActiveUsers(undefined, ...) returns 0, so both cards asserted a confident "0 tjedno aktivnih korisnika" while the request was still in flight or had failed, sitting right next to the users table's own error message. Zero was indistinguishable from unknown. Notes: - Review finding 5 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Flagged independently by all three engines, the joint-highest consensus in the review. - Error copy matches the users table so the panel reads consistently.
Changes: - Track isDefaultWindow and show the specific title plus abbreviation only then - Fall back to the generic "Aktivni korisnici" for any other window The heading was fixed while the window is user-selectable, so picking "Zadnjih 90 dana" left "Tjedno aktivni korisnici (WAU)" sitting over a 90-day number, and both cards could be set to the same window and show the identical figure under two different labels. Notes: - Review finding 38 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md)
Changes: - Add formatDateTime to utils/strings, reusing formatDate for the date half - Add a RelativeTime component that guards an unparsable value and waits for mount - Render the last-login cell through it formatRelativeTime has no invalid-date guard: it falls through its DIVISIONS loop on NaN and calls Intl.RelativeTimeFormat.format(NaN, "years"), which throws a RangeError and takes down the client tree. useGetAllUsers returns raw axios data with no zod parse, unlike the contact and digital-card services, so an unparsable lastLoginAt really can reach the cell. The same line also read Date.now() during the server render with no mount guard, so the relative label could mismatch on hydration and then never refresh. RelativeTime follows the last-synced-label precedent and uses useMinuteTick, rendering the absolute timestamp until mounted. Notes: - Review findings 7 and 39 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - The hydration half was not in the review; it was found while fixing finding 7. - The tooltip now carries the time too, which finding 39 asked for: formatDate emits only dd.mm.yyyy., so hovering "prije 5 minuta" revealed nothing.
Changes: - Split the shared comment so each field describes itself - Note that lastActiveAt is present on every UserDto, not just the admin list The single "Admin list only" comment covered both fields but was true only for lastLoginAt: convertToUserDto sets lastActiveAt, so GET and PATCH /api/users/me return it too. The comment is the only contract doc for these fields, so it would have sent the next consumer to the admin list for data /me already has. Notes: - Review finding 27 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md)
Changes: - Point admin-activity-card and user-activity at @/lib/api/schemas/auth-user The dashboard was split two-vs-two: the newer WAU/MAU files went through the @/lib/api/types barrel while the user-table files imported the schema module directly. Both resolve to the same type, but one feature folder using two paths for one type is churn waiting to happen. Notes: - Review finding 40 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Only the dashboard is normalised here. The contact-side files also use the barrel, but they are outside this review's scope.
![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: 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 `@backend/pom.xml`:
- Around line 25-27: Update the Lombok dependency declaration and its annotation
processor configuration to both use the existing ${lombok.version} property,
ensuring dependency compilation and annotation processing share the same
version.
In `@backend/src/main/java/disscount/user/service/UserService.java`:
- Around line 283-290: Update the last_active_at migration flow around
UserService.toUtcInstant so pre-UTC rows are converted using the prior
deployment’s host zone before being interpreted as UTC. Apply this backfill or
cutover-compatible representation only to historical values, while preserving
UTC handling for values written after the migration and keeping active-user
calculations consistent.
🪄 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: aa6109eb-5a46-4524-872e-6648c524872a
📒 Files selected for processing (7)
.github/dependabot.ymlbackend/pom.xmlbackend/src/main/java/disscount/user/dao/AuthIdentityDao.javabackend/src/main/java/disscount/user/domain/User.javabackend/src/main/java/disscount/user/dto/AuthIdentity.javabackend/src/main/java/disscount/user/dto/UserDto.javabackend/src/main/java/disscount/user/service/UserService.java
Comment on lines
+25
to
+27
| <!-- Overrides the version spring-boot-dependencies manages, and is the single | ||
| source for both the dependency and the annotation processor path below. --> | ||
| <lombok.version>1.18.46</lombok.version> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Apply ${lombok.version} to the Lombok dependency too.
The dependency declaration still has no <version>, so the new property only controls the annotation processor. Maven may therefore compile against Spring Boot’s managed Lombok version while processing annotations with 1.18.46, contradicting the “single source” configuration and risking version-specific build failures.
Proposed fix
<dependency>
<groupId>org.projectlombok</groupId>
<artifactId>lombok</artifactId>
+ <version>${lombok.version}</version>
<scope>provided</scope>
</dependency>Also applies to: 76-80
🤖 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 `@backend/pom.xml` around lines 25 - 27, Update the Lombok dependency
declaration and its annotation processor configuration to both use the existing
${lombok.version} property, ensuring dependency compilation and annotation
processing share the same version.
Comment on lines
+283
to
+290
| .lastActiveAt(toUtcInstant(user.getLastActiveAt())) | ||
| .build(); | ||
| } | ||
|
|
||
| // The column is zone-less but written by nowUtc(), so UTC is the offset it was stamped with. | ||
| private Instant toUtcInstant(LocalDateTime value) { | ||
| return value == null ? null : value.toInstant(ZoneOffset.UTC); | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Migrate pre-UTC last_active_at rows before assigning UTC.
Line 289 reinterprets every existing zone-less value as UTC, but prior releases wrote these values using the host default zone. On non-UTC deployments, historical activity instants—and therefore active-user counters—shift by that offset. Backfill existing rows using the prior deployment zone (or preserve a cutover-compatible representation) before this API change.
🤖 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 `@backend/src/main/java/disscount/user/service/UserService.java` around lines
283 - 290, Update the last_active_at migration flow around
UserService.toUtcInstant so pre-UTC rows are converted using the prior
deployment’s host zone before being interpreted as UTC. Apply this backfill or
cutover-compatible representation only to historical values, while preserving
UTC handling for values written after the migration and keeping active-user
calculations consistent.
Changes: - Move ILabeledSelectOption to @/typings and make it generic over TValue - Type options as readonly ILabeledSelectOption<TValue>[] - Resolve the picked option instead of casting the raw string back to TValue - Type each call site's array against its own union, adding ActivityWindowDays options accepted any string while onValueChange promised TValue, and the gap was closed by an unchecked `next as TValue`. A typo like "totl" compiled cleanly and arrived at setStoreOptimizeMode as a mode that silently never matches anything. Moving the type also removes the coupling that made a dashboard util import from a component module; AGENTS.md puts shared UI types in @/typings. Notes: - Review findings 13 and 32 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Finding 13 was flagged by all three engines. - The default type parameter keeps ILabeledSelectOption usable unparameterised.
Changes: - Generate a useId for the label span - Reference it from SelectTrigger via aria-labelledby The label was only visually adjacent, so the Radix role="combobox" trigger took its accessible name from the current value. A screen reader announced "Trgovinama, combobox" with no indication of what the control does, at every call site. On the dashboard it was worse: two sibling cards each announced only their selected period, with nothing tying either to WAU or MAU. Notes: - Review finding 14 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Flagged by all three engines, the broadest consensus in the review. - aria-labelledby rather than aria-label so the announced name stays in sync with the visible text.
Changes: - Put justify-end back on the wrapper and accept a className override - Restore the trigger's w-full sm:w-60 and size="sm" - Drop the comment explaining the min-h workaround it no longer needs Extracting sort-select into LabeledSelect silently lost three layout classes that both original call sites had: the control stopped being right-aligned, stopped filling the width on mobile, and grew from the small trigger to the default height. That is a visible regression on the product detail page and the shopping-list stores panel. The activity cards opt out of the right alignment, since they sit under a left-aligned number rather than above a list. Notes: - Review findings 12 and 36 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Only Codex caught this one. - Verified the wrapper and trigger classes now match main, modulo the tailwind plugin's class ordering.
Changes: - Restore the sort wording on the product detail chain control The LabeledSelect extraction swapped in "Optimiziraj po:", borrowed from the shopping-list optimize control, which is a different concept. This picks how the chains carrying one product are ordered, and its options are still the dative "Trgovinama / Cijeni / Udaljenosti" written for "Sortiraj po", so the control mislabelled itself on every product detail page. Notes: - Review finding 11 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Keeps the trailing colon, which is now the convention for every label rendered through LabeledSelect.
Changes: - Change the child skeletons from h-7 to h-8 The real child row is SidebarFilterMenu, whose collapsible trigger is a plain SidebarMenuButton at the default h-8, not a SidebarMenuSubButton at h-7. Every child row therefore jumped 4px when the Suspense boundary resolved, which is exactly what the file's own "Mirrors the real row heights so nothing shifts" comment promises it does not do. Notes: - Review finding 17 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md)
Changes: - Add SidebarProductNavShell holding the group, label and menu wrapper - Render both the real nav and the skeleton through it The group label "Istraži" and the SidebarGroup / SidebarGroupContent / SidebarMenu structure were copy-pasted between the two files, so the skeleton would silently stop matching the real nav as soon as either changed shape. The h-7 versus h-8 mismatch fixed in the previous commit is what that drift looks like in practice. Notes: - Review finding 35 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md)
Changes: - Add useClientSearchParams, a client-only query-string read - Use it in SidebarProductNav so the group no longer bails out of prerendering - Wrap only the filter menus in Suspense, with a new SidebarFilterMenuSkeleton - Drop the whole-group boundary from AppSidebar and delete its skeleton SidebarProductNav called useSearchParams purely for active-state and filter params, so the Suspense boundary around it put a skeleton, rather than the Popusti / Karta / Statistika / Novosti anchors, into the static HTML. Those hrefs come from a constant and need no request data, so this partly undid the crawlable-nav fix in c11ea04. The anchors now render statically and only SidebarFilterMenu, which genuinely needs the URL, sits behind a boundary. Notes: - Review findings 16, 17 and 35 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Agreed trade: the ?discounted=true highlight on Popusti now resolves just after hydration instead of in the first paint. Navigating from /products to /products?discounted=true keeps the same pathname, so the highlight there settles on the next navigation; every other entry point is correct. - This supersedes the whole-group skeleton that findings 17 and 35 had just fixed and deduplicated. The h-8 correction from 17 carries over into SidebarFilterMenuSkeleton, which is the only skeleton left.
…e extremes Changes: - Add getChainAvgPriceRange to product-price-utils and call it from all three duplicates (store-chain-extremes, shopping-list-items-table-utils, store-sorting-utils) - Memoise productsData through useQueries' combine option - Delete findCompleteStoresAnalysis, extremeByAvgPrice, ICompleteStoresAnalysis and the prop threading behind them - Give the store-card header arrows aria-hidden plus sr-only text - Split the price row into StoreCardPriceStat and add an sr-only extreme label - Put the optimize option labels in the locative The "parse every chain's avg_price, drop NaN, take min/max" block existed three times with three different empty-case behaviours, which is exactly how the badge and cell rules drifted apart before. productsData was rebuilt outside any memo, so it took a new identity every render and invalidated the allChains, cheapestCountByChain and storesWithHighestPriceItems memos plus the two memo() components downstream. completeStoresAnalysis was computed, threaded through two components and never destructured, implying a "best basket" marker that was not wired up. Cheapest and most-expensive were signalled by icon and colour only, with no text alternative, even though shopping-list-item-row already sets that precedent; the price row was colour-only, which fails WCAG 1.4.1. Notes: - Review findings 20, 21, 22, 23, 33, 36, 37, 40 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - combine is the repo's first use of that option; it is memoised by TanStack. - computeAbsolutePrices and chainsStockingEveryItem are still live and untouched. - "Optimiziraj po:" governs the locative, so three of the four labels were in the wrong case.
Changes: - Raise the sharp devDependency to ^0.35.3 via pnpm add - Note in AGENTS.md that kysely is a direct dep only to pin better-auth's peer - Rewrite the stale DEPLOYMENT.md row that said pnpm overrides are not honored The sharp override declares everything below 0.35.0 unsafe and forces ^0.35.3, but the direct devDependency still asked for ^0.35.2, which the selector does not match. That copy sat outside the override's protection and landed on a safe version only by luck of "latest". Three reviewers independently read kysely as dead weight because nothing imports it. It is not: @better-auth/core declares kysely as a NON-optional peer and the root entry is what satisfies it, with drizzle-orm resolving its own optional peer against the same copy. Removing it would not break the build, since auto-install-peers defaults to true, but the version would float within "^0.28.5 || ^0.29.0" on an auth-critical package. The DEPLOYMENT.md gotcha was the source of that confusion: it told readers overrides do not work and to use a direct dependency instead. Notes: - Review findings 3, 24 and 25 (reviews/REVIEW-2026-07-24-DEV-VS-MAIN.md) - Finding 24 asked for kysely to be removed. Rejected after checking the peer declarations; the real defect was the missing rationale, which is now recorded.
Changes: - Collapse the multi-line explanatory comments added across this branch - Run prettier over three shopping-list files the batch gate missed AGENTS.md asks for single-line comments only where genuinely needed, and says a multi-line explanation usually means the code should be rewritten instead. Several comments introduced earlier on this branch broke that. Notes: - Comment-only for the backend; no behaviour change.
This was referenced Jul 24, 2026
OffCrazyFreak
marked this pull request as ready for review
July 24, 2026 21:49
…chParams Changes: - AUTH.md: add AuthIdentityDao to the key-files map and note UserService stamps UTC - AUTH.md: add three gotchas covering the naive-timestamp stack, why the two dashboard fields are Instant, and that lastLoginAt is derived rather than stored - STATE-PERSISTENCE.md: expand the useSearchParams gotcha to explain that the Suspense fallback is what gets prerendered, and point at useClientSearchParams - STATE-PERSISTENCE.md: correct the stale next version in the library table The docs described the code as it was before this branch. The timezone fix and the sidebar prerender fix both turned on behaviour that was written down nowhere, which is why each took a full review to surface. Notes: - Docs only, no code change.
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.
Implements the findings selected from the 7-reviewer sweep of
devvsmain(5 Claude Opus 5 high, Codexgpt-5.6-solhigh, CodeRabbit; Cursor was out of usage). 40 findings were ranked; 31 fixed here, 6 filed as issues, 1 rejected, 2 informational.One commit per finding early on, then per logical batch. No behaviour is changed beyond what each finding describes.
Area 1: security and supply chain
update-typesfilter, so it suppressed security updates too.SecurityConfigserves/api-docs/**and/swagger-ui/**withpermitAll(), so an unauthenticated Swagger surface pinned at 2.2.0 would never get a security PR. Now bounded atversions: [">=2.3.0"].sharpdevDependency raised to^0.35.3so its declared floor matches the security floor its own override asserts.Area 2: correctness, time and data
The highest-confidence finding in the review, reached independently by three reviewers from both ends.
UserDto.lastLoginAt/lastActiveAtwereLocalDateTime, so Jackson emitted"2026-07-24T13:05:11.123"with no offset and the browser parsed it as local time. In Zagreb a just-signed-in user rendered as "prije 2 sata", and the WAU/MAU window shifted by each admin's UTC offset.last_active_atwas stamped withLocalDateTime.now()(JVM zone) but compared againstMAX(session.updated_at), which drizzle writes in UTC. AnowUtc()helper makes the stored values genuinely UTC.AuthIdentitytoInstant, so the wire format carriesZ. The zod schemas stayz.string(), so parsing is unaffected.Timestamp,Instant,OffsetDateTimeandLocalDateTimeand logs anything else.0while the query is pending or failed.formatRelativeTimehas no invalid-date guard andRangeErrors onNaN, anduseGetAllUsersreturns raw axios data with no zod parse.toInstantconverts aTimestampviatoLocalDateTime()first on purpose: the driver reads a zone-less column in the JVM zone, so callingtoInstant()directly would re-apply that offset.Area 3: LabeledSelect refactor regressions
The
sort-selecttolabeled-selectextraction changed behaviour at its call sites.justify-end,w-full sm:w-60andsize="sm", so all three selects lost right alignment and mobile full width and grew taller. Verified the classes now matchmain.optionsaccepted anystringwhileonValueChangepromisedTValue, closed by an unchecked cast. Now generic, and each call site is typed against its own union.ILabeledSelectOptionmoved to@/typings, removing a util-to-component import.Area 4: sidebar
16
SidebarProductNavcalleduseSearchParamspurely for active state, so the Suspense boundary put a skeleton rather than thePopusti/Karta/Statistika/Novostianchors into the static HTML, partly undoing c11ea04. A newuseClientSearchParamskeeps the group in the prerender, and onlySidebarFilterMenusits behind a boundary now.Agreed trade: the
?discounted=truehighlight resolves just after hydration. This supersedes findings 17 and 35, which had just fixed and deduplicated the whole-group skeleton; theh-8correction carries into the newSidebarFilterMenuSkeleton.Area 5: shopping lists
avg_price, drop NaN, take min/max" block existed three times with three different empty-case behaviours. OnegetChainAvgPriceRangehelper now.productsDatatook a new identity every render, invalidating four memos and twomemo()components. Now built throughuseQueries'combine, the repo's first use of it.completeStoresAnalysiswas computed, threaded through two components and never destructured. Deleted, along with its helper and type.computeAbsolutePricesandchainsStockingEveryItemstay.Area 6: dependencies and docs
Finding 24 was rejected as written. The reviewer called the direct
kyselydependency dead weight because nothing imports it. It is not:@better-auth/core@1.6.25declareskyselyas a non-optional peer and the root entry is what satisfies it, withdrizzle-ormresolving its optional peer against the same copy. Removing it would not break the build, sinceauto-install-peersdefaults to true, but the version would float within^0.28.5 || ^0.29.0on an auth-critical package. The real defect was the missing rationale, now recorded inAGENTS.md.25 rewrites the
docs/DEPLOYMENT.mdrow that claimedpnpm-workspace.yamloverrides "weren't honored" and told readers to use a direct dependency instead. That stale row is what madekyselylook unexplained in the first place; five overrides are live and the lockfile proves they apply.Rejected
Codex's only backend finding claimed
last_active_atwas added without a migration.application.properties:11setsspring.jpa.hibernate.ddl-auto=update, so Hibernate adds the column itself.Filed as issues instead
#115 (pooled price range colours the wrong chain), #116 (badge vs table item sets), #117 (last login vanishes on sign-out), #118 (CI
permissions:block), #119 (future timestamps inflate WAU/MAU), #120 (locked items prefetch six routes).#115 and #116 are pre-existing: the shopping-list reviewer widened past the branch diff, and only three files under
shopping-lists/actually changed betweenmainanddev.Verification
prettier --checkandtsc --noEmitclean across the branch, ignoring the knownPageProps/RouteContexttypegen noise.mvn -B verifycompiled theInstantchange and the DAO extraction early; it passed.Not yet verified on
dev.disscount.me: the timezone fix, the select layout at 375px, and the sidebar anchors in prerendered HTML.