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.jsonpasses 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: translateXwith 150ms transition (reasonable, respects reduced motion implicitly, actually, note: does NOT checkprefers-reduced-motion; minor). globals.cssadds drawer hover rules + hamburger/CTA visibility toggles under the existing@media (max-width: 860px)block. Non-invasive.HeroImage.tsxwas 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/imagewill 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]
priorityon 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. Passalt=""directly since the image is purely decorative. Minor a11y cleanup. - [NIL] No
prefers-reduced-motionguard on the drawertranslateXtransition. 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
useEffectguards (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 --noEmitpasses on the branch.parlay.tsmath 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.tsxactually 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-builderPRODUCT entry) so the merge is essentially a no-op collision.sitemap.tsentry added withchangeFrequency: "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 auseReforcrypto.randomUUID(). - [LOW] Inline smoke tests run at module import:
smokeTest(...)calls at the bottom ofparlay.tsexecute in the production bundle. They onlyconsole.erroron failure (no throw), but they still run a dozen math ops on every client-side import of the module. Gate withif (typeof process !== 'undefined' && process.env.NODE_ENV !== 'production')or move to a siblingparlay.test.ts. - [LOW] plus or minus 100 American odds rejected in UI but accepted in lib:
americanToDecimal(100) === 2.0works fine in the library, butParlayBuilder.tsxrejectsamerican === 100 || american === -100as invalid. Inconsistent, not user-breaking (nobody posts even money in a parlay often) but minor. - [LOW] Large single component (683 lines):
ParlayBuilder.tsxbundles 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). buildProfileRowmapping vsstore.getProfileshape: correct foruserId,username,credits,streak,longestStreak,wins,losses,units,roi,xp,tier,joinedAt,badges. Theclv(memory) vsclv_avg(schema) rename is handled correctly in the read direction.index.jsasync ClientReady:Events.ClientReadycallback markedasync, 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,_exclusionsunderscore-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:127writesexclusionsMap.set(row.discord_id, 'permanent')for permanent exclusions.store.js:181 to 185:isExcluded(userId)doesnew Date(until) > new Date().new Date('permanent')returnsInvalid Date;Invalid Date > new Date()isfalse.- 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):
- In
isExcluded, test for the sentinel first:if (until === 'permanent') return true; - In
hydrate.js, store permanent exclusions asNumber.POSITIVE_INFINITY(matches the timestamp type the Map otherwise holds); thennew Date(Infinity) > new Date()is truthy. Still ugly, option 1 is cleaner. - 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 bysetExclusion.
- In
- My recommendation: option 1 + option 3 together (defense in depth). Add the
'permanent'check inisExcludedand 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
clvtoclv_avgone-way: hydrate readsclv_avgintoclv, butupdateProfileinstore.jsdoes not mirrorclvwrites back toclv_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. CurrentDEFAULT_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
+1PRODUCT 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-builderlink in mobile drawer: The drawer iteratesGROUPS->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 safeuseStateinitial values. No SSR hydration risk.
Must-fix-before-merge items
- Branch C permanent-exclusion sentinel bug: do NOT merge C to main without either (a) fixing
isExcludedor (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
- [HIGH]
store.isExcludeddoes not handle the permanent flag. This is not a bug Agent C introduced; the in-memory-onlysetExclusionpath also storesnullforuntilTimestampwhenpermanentis true, andnew Date(null).getTime() = 0, sonew Date(0) > new Date()isfalse. Permanent exclusions set via/exclude permanentin 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. - [LOW]
clvvsclv_avgone-way sync (see Branch C findings). Queue as "Phase 3: complete write-through for clv, joined_at, badges". - [LOW]
next/image+ aria-hidden + non-empty alt onHeroImage(see Branch A findings). Queue as "A11y: decorative hero images should use alt=''".
5. Follow-up Recommendations for the Next Autonomous Burn
- 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. - 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.
- Split
ParlayBuilder.tsx(683 lines) intobooks.ts+LegRow.tsx+BookCard.tsxwhen next touched. Not urgent. - Gate inline smoke tests in
parlay.tsbehindNODE_ENV !== 'production'or move to a.test.tsfile. - 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. ✓