Skip to content

fix(02): resolve Phase 2 code-review findings (CR-01, WR-01..03) - #16

Merged
alexandermayes merged 7 commits into
mainfrom
growth/phase-2-review-fixes
Sep 14, 2026
Merged

alexandermayes merged 7 commits into
mainfrom
growth/phase-2-review-fixes

Conversation

@alexandermayes

Copy link
Copy Markdown
Owner

Summary

Resolves all four findings from the Phase 2 code review (02-REVIEW.md):

  • CR-01 (critical)generateSuggestions emitted numeric healer overheal/uptime advice for healers with hasHealing: false (zero recorded healing = "no data" everywhere else). Now gated on hasHealing; regression test added. This was a live wrong recommendation on parseforge.gg.
  • WR-01/api/timeline's "player not found in fight" check used the report-wide actor list. Now uses playerDetails(fightIDs:), matching /api/analyze.
  • WR-02 — PostHog timeline_filter_used capture lived inside a setState functional updater and double-fired under Strict Mode. Moved out of the updater.
  • WR-03 — Corrected stale era-precedence comment in game-data.test.ts (production rule is Classic/TBC-first) and added a test pinning the real ENCHANT_NAME_DB collision behaviour.

Also: 02-REVIEW-FIX.md report, and a pending todo capturing World of Warcraft Forever support for when WCL exposes Forever logs.

Verification

  • npx tsc --noEmit clean
  • npx vitest run 137/137 (baseline 135; +2 regression tests)
  • npm run lint total unchanged vs main (pre-existing debt only)

Deploy

Manual Vercel CLI — preview first, prod after owner confirmation.

🤖 Generated with Claude Code

alexandermayes and others added 5 commits September 14, 2026 10:58
…dvice on zero-healing data

Add regression test asserting a healer with hasHealing: false yields no
healer-category suggestions.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
…ght-scoped

masterData.actors is the report-wide roster; a sourceId from a different
fight in the same report passed the old check. Fetch playerDetails(fightIDs:
[fightId]) (as app/api/analyze/route.ts already does) and check membership
there instead.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
…meline

toggleAbility fired onFilterToggle (timeline_filter_used) from inside the
setHiddenAbilityIds functional updater, which React Strict Mode invokes
twice — double-firing the event on every ability-chip click in dev.
Compute the toggled set up front and call setHiddenAbilityIds with the
concrete value instead, mirroring resetAbilities.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
The local composeEras() helper's comment claimed later-era-wins 'matches
current behaviour', but lib/generated/index.ts's composeEraPriority
implements the opposite (first-resolved-era-wins). Correct the comment and
add Test 9, asserting the real production ENCHANT_NAME_DB resolves the
known id 3003 collision to the Classic/TBC fact, not WotLK's — closing the
coverage gap the stale comment papered over.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@vercel

vercel Bot commented Sep 14, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
parseforge Ready Ready Preview Sep 14, 2026 8:21pm UTC

alexandermayes and others added 2 commits September 14, 2026 12:21
…agnosis, context, research, validation strategy

PostHog capture has been ~99.9% down since the Phase 1 deploy (2026-09-06):
cookieless_mode "on_reject" drops all PENDING-consent events and opt-in
depended on __tcfapi calling back, which never happens for fresh visitors.
Phase 2.1 (INSERTED after Phase 2) fixes it with server-side geo and hardens
the OPS-01 gate to require post-deploy live-traffic evidence.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
@alexandermayes
alexandermayes merged commit 66b59da into main Sep 14, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant