Comments by rowm30
All comments ranked by humor rating
You were supposed to fix the build.
Instead, you’ve returned with… this. A sprawling, epic feature commit that not only ignores the previous review but also introduces a user identity system seemingly designed by a TikTok influencer. I’m not angry, I’m just… bewildered.
The Build is Still a Dumpster Fire 🔥 Let’s start with the most offensive part. The build is still broken. With the exact same errors I pointed out last time.
Cannot find name 'jest'
Property 'ip' does not exist on type 'NextRequest'
Property 'errors' does not exist on type 'ZodError'
Did you think the CI pipeline would just give up and fix the code itself? Not only did you not fix the Zod error, you copied the same bug into your new api/user/upsert endpoint. This is a masterclass in ignoring feedback. I'm taking notes.
The "Vibey-Goat" User Journey While the application is metaphorically on fire, you were busy building a whole new user identity system. And I have to talk about the wordlists.
genz_adjectives.ts: ['vibey', 'cringe', 'sus', 'rizz', 'yeet', 'savage'] genz_nouns.ts: ['goat', 'vortex', 'baddie', 'wolf', 'wizard', 'boss']
I had to get up and walk around the room after reading this. Are we building a secure application or a meme generator? I can't wait to greet our first enterprise customer, Savage-Baddie, or debug an issue for user Cringe-Goat. This is, without a doubt, the most insane design choice I have ever seen in my professional career. It's so audacious I almost respect it. Almost.
A Word on Commit Scope You called this commit "Implement user journey flow and fix index warning." This isn't a commit; it's a novel. You added an entire user identity system, a new database model, multiple API endpoints, several new pages, and middleware logic... and you bundled it with a one-line change to a Docker script. This is pure chaos.
Conclusion I'm not even looking at the rest of this. Here is your new priority list:
FIX. THE. BUILD. Address every single error from the last review.
Go back to the drawing board on the user display names, unless our target audience is literal children.
Learn to break your work down into smaller, logical pull requests.
Do not come back until the CI is green. I need to go lie down.
Ah, a "fix" commit. My favorite. Especially one that attempts to solve a subtle crypto issue while simultaneously breaking the build in three different ways. It's a bold strategy.
The Actual Fix in keyManager.ts Let's start with the part that isn't on fire. The change to storePrivateKey and initializeDevice is actually a clever workaround for the Web Crypto API's charming inflexibility. Realizing you can't re-export a non-extractable key after it's been stored, you've opted to save the JWK representation upfront. Manually re-building the public key from the private JWK components is a nice touch. I'm almost impressed. You've fixed a genuinely tricky bug.
Now, about the cost of that victory...
Let's Talk About All That Red Your CI pipeline is screaming, and frankly, I don't blame it. It looks like you've introduced a spectacular collection of regressions.
Cannot find name 'jest' In AuthButton.test.tsx, you're using Jest's types without, apparently, telling TypeScript what Jest is. It's a revolutionary concept, but your test files usually need access to test-related type definitions. Maybe add @types/jest to your devDependencies or check your tsconfig.json? Just a wild guess.
Property 'ip' does not exist on type 'NextRequest' This error is littered across every API route. Did we travel back in time to an older version of Next.js? request.ip has been gone for a while. You're already using request.headers.get('x-forwarded-for') as a fallback, so you're so close. Just delete the part that's broken.
Property 'errors' does not exist on type 'ZodError' I know, keeping up with library updates is hard. A quick look at the Zod documentation would reveal that the property is now issues, not errors. But who has time to read docs when you're busy fixing things?
Conclusion So, to summarize: you fixed a subtle, complex crypto bug by introducing several loud, simple build errors. It's like fixing a leaky faucet by knocking a hole in the wall.
Fix the build. Then we'll talk about merging this "fix."
I'm speechless. Truly.
You have submitted a third pull request. You have refactored the key management logic. You have addressed a race condition in a React component. And the build is still broken. With the exact same errors as before.
This has gone from funny to performance art.
The Pointless but Beautiful Refactor Let's start with keyManager.ts. You've extracted the IndexedDB logic into clean idbGet and idbPut helpers. You wrote a lovely, idempotent ensureDeviceKeys function. This is good, clean, thoughtful code.
And it is utterly and completely pointless while the CI checks are failing. This is like meticulously polishing the chrome on a car that has no engine. It looks shiny, but it's not going anywhere.
The React Component Museum Over in LandingPreview.tsx, I see you've added a mounted flag to your useEffect. A true classic from the React archives, a solution to a problem that modern hooks and AbortController handle more elegantly. It’s like seeing someone use a flip phone in 2025. It’s quaint. Nostalgic, even.
And the dynamic import() for the key manager? A bold choice. Are we so concerned with the bundle size of our landing page that we need to code-split the most critical piece of client-side logic it needs to function? It feels like you're trying to solve a performance problem we don't have while ignoring the sea of red ink in the logs.
The Thing You Actually Need To Do This is the third time I am writing this. The build is failing.
Cannot find name 'jest'
Property 'ip' does not exist on type 'NextRequest'
Property 'errors' does not exist on type 'ZodError'
You have now written and refactored features on top of a codebase that does not compile.
I am not reviewing another line of code. I don't care about your clever refactors or your Gen-Z display names.
Close this PR. Open a new, single-commit PR that does nothing but make the build pass. That's it. Once the light is green, and only then, can we discuss merging anything else.
This is no longer a suggestion.