← Docs

Agent Cross-Review, April 18, 2026

Reviewer: architect-reviewer (read-only, no merge/push executed)


1. Executive Verdict

Branch Verdict Blocking
agent-a-mobile-nav-heroes APPROVE WITH FIXES (non-blocking) None that block merge. Hero image byte sizes should be optimized, but visual/functional correctness is fine.
agent-b-parlay-builder APPROVE WITH FIXES (non-blocking) None that block merge. Two minor code-quality items (module-level mutable state, inline smoke tests running in prod bundle).
agent-c-persistence-phase2 APPROVE WITH FIXES, ONE MUST-FIX store.isExcluded() vs hydrated 'permanent' sentinel silently disables self-exclusion after restart. This is a responsible-gambling safety regression. Fix before merge OR fix immediately after in a hotfix commit on the same PR.

2. Per-Branch Findings

Branch A: agent-a-mobile-nav-heroes

Verified claims

  • tsc --noEmit -p web/tsconfig.json passes cleanly on the branch (with main's node_modules symlinked).
  • SiteNav rewrite converts file to "use client", adds focus trap, Escape handler, body scroll lock, aria-modal="true", aria-expanded, aria-controls. All implemented.
  • Drawer uses transform: translateX with 150ms transition (reasonable, respects reduced motion implicitly, actually, note: does NOT check prefers-reduced-motion; minor).
  • globals.css adds drawer hover rules + hamburger/CTA visibility toggles under the existing @media (max-width: 860px) block. Non-invasive.
  • HeroImage.tsx was already present on main; not a new component. A just consumes it on 5 pages.

Issues / risks

  • [LOW] Hero images are 2.0 to 3.0 MB each (12 MB total new binary commit). next/image will generate responsive variants but the source files are oversized. Recommend running through image optimizer (target ~300 to 500 KB per source JPEG). Not a merge blocker (Vercel won't serve the raw source in production), but the git repo is now ~12 MB heavier than it needs to be.
  • [LOW] priority on every hero <Image>: only one page renders per navigation, so this is fine in practice, but stylistically "every hero is priority" means no meaningful priority signal.
  • [LOW] Decorative image semantics: the <div aria-hidden="true"> wraps an <Image alt="Three neural nodes connected by luminous data streams">. If the wrapper is aria-hidden, the alt text is functionally hidden to AT. Pass alt="" directly since the image is purely decorative. Minor a11y cleanup.
  • [NIL] No prefers-reduced-motion guard on the drawer translateX transition. DESIGN.md implies reduced-motion compliance; recommend adding a CSS media query.
  • No em dashes added. No partner name drops. Neural Dark tokens (#00FF88, #080810, #1F1F35, #F0F0F5, #6B7294, #9AA0B4, #8B5CF6, #FF2D7B, #FFD700) are consistent with DESIGN.md.

SSR hydration

  • useState(false) initial matches server render (drawer closed). No hydration mismatch risk.
  • The useEffect guards (if (!drawerOpen) return) prevent DOM-access during SSR. Safe.

Focus trap verified by code reading

  • Tab-forward wraps to first, Shift+Tab-backward wraps to last: correct.
  • Focus is initially set to first focusable element on drawer open: correct.
  • Escape closes drawer AND returns focus to hamburger (hamburgerRef.current?.focus()): correct.
  • Body scroll locked via document.body.style.overflow = "hidden" with cleanup on unmount: correct.

Branch B: agent-b-parlay-builder

Verified claims

  • tsc --noEmit passes on the branch.
  • parlay.ts math independently verified:
    • Two -110: 1.9091 x 1.9091 = 3.6446 -> (3.6446 - 1) x 100 = +264
    • +200 + +150: 3.0 x 2.5 = 7.5 -> +650
    • -300 + -150: 1.3333 x 1.6667 = 2.2222 -> +122 ✓ (correct fair-odds result; the "-105" value commonly cited is wrong)
  • SiteNav.tsx actually underwent the same full rewrite in B as in A (hamburger + drawer + focus trap). Agent B's claim that this was a "one-line edit" is incorrect. However, this is fortunate: A and B differ by only one comment line (plus B's /parlay-builder PRODUCT entry) so the merge is essentially a no-op collision.
  • sitemap.ts entry added with changeFrequency: "weekly", priority: 0.9: consistent with sibling landing pages.
  • Metadata: canonical, OG, Twitter card, keywords, breadcrumb structured data all present and clean. SEO package is solid.
  • Deep-link URL patterns reviewed (not tested live): DraftKings /leagues/football/nfl, FanDuel /navigation/football/nfl, BetMGM /en/sports/football/nfl, Caesars /us/bet/football/nfl, Pinnacle /en/football/matchups, Circa -> homepage (acknowledged). These are plausible and will open the right sport hub even if the exact slug for a given league shifts (bookss periodically rename paths). Acceptable for now; flag as a future maintenance item.

Issues / risks

  • [LOW] Module-level mutable state: let nextId = 1; at module scope means leg IDs do not reset across component remounts within the same JS runtime. Not user-visible but unclean. Move to a useRef or crypto.randomUUID().
  • [LOW] Inline smoke tests run at module import: smokeTest(...) calls at the bottom of parlay.ts execute in the production bundle. They only console.error on failure (no throw), but they still run a dozen math ops on every client-side import of the module. Gate with if (typeof process !== 'undefined' && process.env.NODE_ENV !== 'production') or move to a sibling parlay.test.ts.
  • [LOW] plus or minus 100 American odds rejected in UI but accepted in lib: americanToDecimal(100) === 2.0 works fine in the library, but ParlayBuilder.tsx rejects american === 100 || american === -100 as invalid. Inconsistent, not user-breaking (nobody posts even money in a parlay often) but minor.
  • [LOW] Large single component (683 lines): ParlayBuilder.tsx bundles state, book list, sport slug mapping, and full UI in one file. Works, but will be harder to maintain. Consider splitting (e.g. books.ts, LegRow.tsx, BookCard.tsx).
  • No em dashes added. No partner name drops.

Canonical + breadcrumb clean; sitemap entry priority correct.


Branch C: agent-c-persistence-phase2

Verified claims

  • node -c src/bot/services/hydrate.js: PARSE OK.
  • node -c src/bot/services/hydrate.test.js: PARSE OK.
  • Ran node src/bot/services/hydrate.test.js: 6 passed, 0 failed (happy path, no DATABASE_URL, no pool, query throws, null result, sparse row).
  • buildProfileRow mapping vs store.getProfile shape: correct for userId, username, credits, streak, longestStreak, wins, losses, units, roi, xp, tier, joinedAt, badges. The clv (memory) vs clv_avg (schema) rename is handled correctly in the read direction.
  • index.js async ClientReady: Events.ClientReady callback marked async, hydration wrapped in try/catch. require('./services/hydrate') lives inside the try block, so a module-load failure is caught cleanly.
  • Graceful degradation: confirmed via smoke tests that (a) no DATABASE_URL -> skipped, (b) getPool() returns null -> skipped, (c) query throws (e.g. relation does not exist) -> logs and returns { ok: false, error } without throwing, (d) null query result -> treated as empty rows.
  • _profiles, _dailyClaims, _exclusions underscore-prefixed exports with a comment flagging them as hydrate-only internals. Acceptable.

Issues / risks

  • [CRITICAL, MUST-FIX] Permanent exclusion sentinel silently breaks self-exclusion.
    • hydrate.js:127 writes exclusionsMap.set(row.discord_id, 'permanent') for permanent exclusions.
    • store.js:181 to 185: isExcluded(userId) does new Date(until) > new Date().
    • new Date('permanent') returns Invalid Date; Invalid Date > new Date() is false.
    • Result: after any bot restart, permanent self-exclusions stop working. This is the one feature that must never fail silently. Responsible gambling is also legally/reputationally sensitive.
    • Fix options (choose one, <=10 lines):
      1. In isExcluded, test for the sentinel first: if (until === 'permanent') return true;
      2. In hydrate.js, store permanent exclusions as Number.POSITIVE_INFINITY (matches the timestamp type the Map otherwise holds); then new Date(Infinity) > new Date() is truthy. Still ugly, option 1 is cleaner.
      3. Best: in hydrate.js, store a far-future timestamp (Date.now() + 1000 * 60 * 60 * 24 * 365 * 100). Backwards-compatible with the existing in-memory shape used by setExclusion.
    • My recommendation: option 1 + option 3 together (defense in depth). Add the 'permanent' check in isExcluded and use a far-future timestamp in hydrate so the rest of the code can keep treating the value as "some kind of timestamp".
  • [LOW] Known caveat clv to clv_avg one-way: hydrate reads clv_avg into clv, but updateProfile in store.js does not mirror clv writes back to clv_avg. Memory drifts from DB after writes. Not blocking Phase 2 (read-through hydration), but queue for Phase 3 (full write-through parity).
  • [LOW] BIGINT coercion: Number(profileRow.credits ?? 1000) silently coerces pg BIGINT strings to Number, which is fine as long as credits stay <2^53. Current DEFAULT_CREDITS=1000, unlikely to overflow. Safe.
  • [NIL] Internal Map exports: acceptable for internal use. Low abuse risk given the _ prefix + comment. Leave as-is.
  • No em dashes added. No partner name drops.

3. Merge Strategy Recommendation

Order

C -> A -> B (or equivalently C first, then A+B in either order).

Rationale:

  • C is orthogonal (only src/bot/, docs/). Zero conflicts with A or B. Ship first, lowest-risk win, gives Phase 2 hydration as soon as Railway restarts.
  • A before B because B "assumes" A's drawer architecture. Even though the files are near-identical, landing A first anchors main with the richer rewrite; B's merge is then just the +1 PRODUCT entry and a one-comment-line cleanup.

SiteNav.tsx conflict

Scope of actual conflict: 1 line, inside the focus-trap useEffect. A has no comment, B has // Focus the close button when drawer opens.

Resolution: keep B's comment (it's harmless documentation) OR drop it. Either is fine. Not worth a sub-discussion.

The PRODUCT array addition is in a different hunk entirely and applies cleanly (A did not modify that line range).

Concrete merge sequence:

git checkout main
git merge --ff-only agent-c-persistence-phase2     # zero conflicts
# apply must-fix for permanent exclusion bug as a SEPARATE COMMIT on main
git merge agent-a-mobile-nav-heroes                # zero conflicts
git merge agent-b-parlay-builder                   # 1 trivial conflict in SiteNav.tsx
# resolve: keep B's comment line, keep B's PRODUCT entry
git add web/src/components/SiteNav.tsx
git commit --no-edit

Cross-branch functional checks

  • /parlay-builder link in mobile drawer: The drawer iterates GROUPS -> PRODUCT.items, so B's PRODUCT entry automatically renders in both desktop dropdown AND A's mobile drawer. ✓
  • Circular dependency risk: None. A and B are both client components using useState; no shared imports other than @/components/SiteNav (which B is modifying into A's version).
  • globals.css conflict: B did not touch globals.css. Confirmed. Only A's diff appears there. ✓
  • SSR / hydration: Both A's SiteNav and B's ParlayBuilder use "use client" with safe useState initial values. No SSR hydration risk.

Must-fix-before-merge items

  1. Branch C permanent-exclusion sentinel bug: do NOT merge C to main without either (a) fixing isExcluded or (b) changing hydrate's sentinel. Even though the current behavior on main is already "no hydration at all, so no bug", the moment hydration ships, a restart silently breaks self-exclusion for any user who ever self-excluded. This is small (3 to 10 line fix) but mandatory.

All other findings are post-merge follow-ups, not blockers.


4. Pre-existing Bugs Surfaced During Review

  1. [HIGH] store.isExcluded does not handle the permanent flag. This is not a bug Agent C introduced; the in-memory-only setExclusion path also stores null for untilTimestamp when permanent is true, and new Date(null).getTime() = 0, so new Date(0) > new Date() is false. Permanent exclusions set via /exclude permanent in a single session don't work today, even without hydration. This is a pre-existing RG safety bug. Queue as its own task, title: "Fix permanent self-exclusion in store.isExcluded (RG safety)". Branch C's fix should cover both paths.
  2. [LOW] clv vs clv_avg one-way sync (see Branch C findings). Queue as "Phase 3: complete write-through for clv, joined_at, badges".
  3. [LOW] next/image + aria-hidden + non-empty alt on HeroImage (see Branch A findings). Queue as "A11y: decorative hero images should use alt=''".

5. Follow-up Recommendations for the Next Autonomous Burn

  1. Fix the RG permanent-exclusion bug IMMEDIATELY (before or as part of the C merge). Title: fix(bot): handle permanent self-exclusion in store.isExcluded + hydrate. 1 commit, <=10 lines.
  2. Compress the 5 new hero JPEGs from 2 to 3 MB down to 300 to 500 KB before pushing to production. Adds no risk to the branch; do it as a follow-up commit on main after A lands.
  3. Split ParlayBuilder.tsx (683 lines) into books.ts + LegRow.tsx + BookCard.tsx when next touched. Not urgent.
  4. Gate inline smoke tests in parlay.ts behind NODE_ENV !== 'production' or move to a .test.ts file.
  5. Process improvement for the 3-agent parallel pattern going forward:
    • Agent B's "MEDIUM merge risk" self-flag was wrong in direction (said one-line edit; delivered full rewrite) but right in outcome (near-zero conflict). Future branches should declare what files they will rewrite in their plan before starting, so the coordinator catches overlap earlier.
    • When two agents are both going to touch the same file, pre-agree on an ownership split. A owning SiteNav.tsx + B returning a patch file (unified diff) instead of a full rewrite would have been cleaner.
    • Hero-image byte-size policy should be part of the agent's acceptance criteria when it's generating assets. "Ship <500 KB per hero" as a hard rule would have caught the 12 MB commit before it happened.
    • For bot-persistence-style safety-critical changes (Branch C's exclusion path), add an explicit acceptance test to the agent's instructions: "Write a test that verifies self-exclusion survives a simulated restart". That would have caught the 'permanent' sentinel bug during agent execution rather than at review.

Appendix A: Verification commands run

git log --oneline main..<branch>            # one commit each, descriptive messages
git diff --stat main..<branch>              # expected file sets
git worktree add + tsc --noEmit             # A passes, B passes
node -c src/bot/services/hydrate.js         # PARSE OK
node -c src/bot/services/hydrate.test.js    # PARSE OK
node src/bot/services/hydrate.test.js       # 6/6 passed
git merge-tree <base> agent-a agent-b       # 1 trivial conflict (one comment line)
git merge-tree <base> agent-a agent-c       # 0 conflicts
git merge-tree <base> agent-b agent-c       # 0 conflicts

Appendix B: Token compliance

All three branches: Neural Dark palette only (#080810, #0C0C18, #12121E, #1A1A2E, #1F1F35, #F0F0F5, #9AA0B4, #6B7294, #00FF88, #8B5CF6, #FF2D7B, #FFD700, #FBBF24). No em dashes. No partner name-drops. ✓

21+ only. Not financial advice. 1-800-GAMBLER.

agent cross review apr18 - NuroPicks Docs