Multi-instance PGCR uses standard PGCR UI - #319
Merged
Conversation
Merge multiple RaidHub instances into one synthetic InstanceExtended so multi sessions use PGCRHeader, PGCRPlayers, and the player details panel like single-instance PGCR. Add mergeRaidHubInstances, session timeline for 2+ instances, and hide reporting when the view is merged. Remove the legacy multi combinator/player/details components. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Mark the shared PGCR view as client-rendered so MultiLoader can render it without tripping RSC/Suspense failures, and add a single Playwright suite plus local multi seeding so the multi PGCR path is exercised end to end. Made-with: Cursor
Run Playwright through bunx in GitHub Actions so the binary resolves correctly, and commit the Prettier output needed for the formatting check. This keeps the seeded multi E2E flow green in CI and locally. Made-with: Cursor
Point the libsql-backed local Prisma adapter at the same sqlite file that migrations and E2E seeding use so Playwright can resolve pgcr_multi during CI and local runs. Made-with: Cursor
Commit the local Prisma adapter path change that CI needs for seeded multi E2E, and ignore the empty repo-root sqlite artifact so local runs stay clean. Made-with: Cursor
Use pathToFileURL for the local sqlite adapter path so libsql initializes correctly on Linux runners while still pointing at the seeded prisma database used by multi E2E. Made-with: Cursor
Use the seeded prisma SQLite path for local APP_ENV runs because the absolute file URL form breaks libsql on the Linux Playwright runner. Made-with: Cursor
Ensure the spawned Next.js server runs with APP_ENV=local so E2E uses the seeded SQLite database instead of trying to initialize the remote libsql client in CI. Made-with: Cursor
Point the local libsql adapter at the seeded prisma database without the leading ./ because the Linux Playwright runner still rejects that native sqlite path shape. Made-with: Cursor
Avoid the libsql adapter for APP_ENV=local so Playwright uses Prisma's built-in SQLite path handling while production still uses the Turso adapter. Made-with: Cursor
Return an empty local manifest in APP_ENV=local when RAIDHUB_API_KEY is unavailable so the E2E app shell can render in CI while API-backed production behavior stays unchanged. Made-with: Cursor
Run Playwright entirely against local fixture-backed PGCR data when CI is missing external RaidHub auth so the seeded single and multi PGCR pages stay testable without live API dependencies. Made-with: Cursor
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+71
to
+77
| function mergeNullableBool(values: readonly (boolean | null)[]): boolean | null { | ||
| if (values.length === 0) return null | ||
| if (values.every(v => v === true)) return true | ||
| if (values.every(v => v === false)) return false | ||
| if (values.every(v => v === null)) return null | ||
| return false | ||
| } |
There was a problem hiding this comment.
Bug: The mergeNullableBool function incorrectly merges mixed true and null values into false, causing incorrect UI states for properties like fresh and flawless.
Severity: MEDIUM
Suggested Fix
Update the mergeNullableBool function to handle mixed values correctly. If an array contains both true and null values, the function should not default to false. It should prioritize true if present, or return null to represent the mixed/unknown state, ensuring the UI can distinguish between a definitive false and an ambiguous null state.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/lib/multi/merge-instances.ts#L71-L77
Potential issue: The `mergeNullableBool` function at
`src/lib/multi/merge-instances.ts:71-77` incorrectly handles merging arrays containing
both `true` and `null` values. It defaults to returning `false` in this scenario. This
affects properties like `fresh` and `flawless`, where `null` represents an "unknown" or
"not applicable" state, while `false` means a definitive "no". For example, merging an
instance with `fresh: true` and another with `fresh: null` results in `false`, causing
the UI to incorrectly display that the activity started from a checkpoint, when the
correct state should be ambiguous or reflect the fresh start.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Pull request overview
This PR updates the multi-instance PGCR flow to render using the standard single-instance PGCR UI by synthesizing a merged RaidHubInstanceExtended and optionally embedding a per-instance session timeline.
Changes:
- Added
mergeRaidHubInstancesto aggregate multiple instance payloads into one synthetic PGCR dataset plus timeline segments. - Updated multi PGCR loading to render the shared
PGCRview (with reporting disabled for merged sessions) and addedPGCRMultiTimelineinside the PGCR card. - Removed the legacy multi combinator/player/weapons list components and replaced the old combined-data types with timeline segment types.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib/multi/multi-types.ts | Replaces legacy combined multi types with MultiInstanceTimelineSegment for the new timeline UI. |
| src/lib/multi/merge-instances.ts | New merge function to synthesize a RaidHubInstanceExtended and timeline from multiple instances. |
| src/hooks/profile/useRaidTags.ts | Reformats an op-item event entry (but retains an invalid date range). |
| src/hooks/pgcr/ClientStateManager.tsx | Adds allowsReporting to PGCR context to support hiding reporting for merged sessions. |
| src/components/pgcr/pgcr-view.tsx | Allows multi rendering via merged data + timeline + RIIS options + reporting toggle. |
| src/components/pgcr/pgcr-multi-timeline.tsx | New/updated timeline component for multi-instance session visualization within PGCR. |
| src/components/pgcr/pgcr-menu.tsx | Hides the Report option when allowsReporting is false. |
| src/components/pgcr/pgcr-header-bg.tsx | Fixes potential crash by guarding variant with optional chaining. |
| src/components/multi/multi-loader.tsx | Loads multiple instances, merges them, and renders shared PGCR view (instead of legacy combinator). |
| src/components/multi/multi-weapons-list.tsx | Removed legacy multi-only weapons list component. |
| src/components/multi/multi-player.tsx | Removed legacy multi-only player component. |
| src/components/multi/multi-combinator.tsx | Removed legacy multi combinator logic/UI. |
| src/app/multi/[multiId]/page.tsx | Passes multiId into MultiLoader for synthetic instance IDs and merge behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| shattershafts: { start: new Date("2026-01-27T17:00:00Z"), end: new Date("2025-02-03T17:00:00Z") }, | ||
| shattershafts: { | ||
| start: new Date("2026-01-27T17:00:00Z"), | ||
| end: new Date("2025-02-03T17:00:00Z") |
Comment on lines
+156
to
+161
| const merged: RaidHubInstanceExtended = { | ||
| ...first, | ||
| instanceId: `multi:${multiId}`, | ||
| completed: sorted.every(i => i.completed), | ||
| flawless: mergeNullableBool(sorted.map(i => i.flawless)), | ||
| fresh: mergeNullableBool(sorted.map(i => i.fresh)), |
Resolve pgcr-header-bg duplicate line and combine allowsReporting with blacklist menu support from main. Co-authored-by: Cursor <cursoragent@cursor.com>
Closed
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
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.
Summary
Renders multi-PGCR sessions with the same components as single-instance PGCR (
PGCRHeader,PGCRPlayers, player details panel, query params).Changes
mergeRaidHubInstancesto build a syntheticRaidHubInstanceExtendedfrom multiple instance payloads.MultiLoadernow renders the sharedPGCRview with merged data; reporting is disabled when the session is merged (no single RaidHub instance id).PGCRMultiTimeline.capTPS: false) to match the previous multi combinator.Testing
Made with Cursor