Add game_id to event_props table

[tudddorrr]

No description provided.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.73%. Comparing base (9aca869) to head (add86dd).
⚠️ Report is 6 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop     #936      +/-   ##
===========================================
- Coverage    97.78%   97.73%   -0.05%     
===========================================
  Files          406      406              
  Lines         6634     6635       +1     
  Branches       884      885       +1     
===========================================
- Hits          6487     6485       -2     
- Misses          64       67       +3     
  Partials        83       83              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[claude]

PR Review: Add game_id to event_props table

Summary: This PR denormalizes game_id, dev_build, and created_at into the event_props ClickHouse table, enabling direct filtering without expensive JOINs or subqueries on the events table. The sort key change to (game_id, created_at, event_id, prop_key) aligns well with the new filtering patterns.

---

1. Code Quality and Best Practices

No issues found. The migration follows the established EXCHANGE TABLES pattern from migration 008 (ReorderEventsSortKey), and the migration runner correctly splits statements on ; so multi-statement migrations work as expected. Query updates are consistent across all call sites.

---

2. Potential Bugs or Issues

No issues found. Results are accurate across all modified queries.

---

3. Performance Considerations

No issues found. The sort key (game_id, created_at, event_id, prop_key) enables efficient block pruning for the query shapes introduced here. Adding game_id as an outer pre-filter in dataExportProcessor.ts step 3 — before the pagination subquery — lets ClickHouse skip irrelevant blocks before evaluating the inner subquery.

---

4. Security Concerns

No issues found. All interpolated values (ctx.state.game.id, dataExport.game.id) are integer IDs sourced from the database. User-supplied strings (search, eventName) continue to use ClickHouse parameterized queries.

---

5. Test Coverage

🔵 Minor — Codecov reports one partial branch in dataExportProcessor.ts

The includeDevData ternary on the new outer event_props filter in the rawProps query (step 3) has a partially covered branch. The test fixes in this PR are a meaningful improvement, but a test exercising the includeDevData = false path through this specific new outer filter would fully close the gap.

---

Notable fixes included in this PR

The setupTest.ts change (adding event_props to the truncation list) and the test insertion corrections (from events to event_props) fix pre-existing test isolation bugs that were causing props-related scenarios to silently pass even when the underlying query was broken.

---

Overall: The PR is well-implemented and the approach is sound. The migration is safe, query updates are consistent across all call sites, and the performance improvements are meaningful. The test coverage gap (1 partial branch) is the only thing left to tighten up.

[claude]

PR Review: Add game_id to event_props table

Overview

This PR denormalizes game_id, dev_build, and created_at from the events table into event_props. The payoff is that queries on event_props can now filter directly using the new MergeTree sort key (game_id, created_at, event_id, prop_key) instead of joining back to events for every lookup. The migration is atomic (CREATE to INSERT via SELECT JOIN to EXCHANGE TABLES to DROP), and the multi-statement SQL is handled correctly by the migration runner's semicolon-split loop.

---

1. Code Quality and Best Practices

No issues found. The denormalisation approach is appropriate for ClickHouse and the pattern is applied consistently across routes and the export processor.

---

2. Potential Bugs

🔵 Minor: dev_build not filtered in the breakdown.ts subquery

When includeDevData is false, AND dev_build = false is appended only to the outer event_props query. The inner subquery still returns dev build event IDs without filtering them. This is not a correctness bug — the outer filter handles it correctly, and the existing tests at lines 213 and 256 of breakdown.test.ts confirm the right data is returned. However, the IN list unnecessarily includes dev build event IDs, which wastes work when dev events are common.

dataExportProcessor.ts already uses the cleaner inline pattern for both the outer and inner queries. Applying the same to the subquery in breakdown.ts would be consistent and marginally more efficient.

---

3. Performance Considerations

No issues found. The new sort key (game_id, created_at, event_id, prop_key) is well-chosen for all the query patterns introduced here. Eliminating the JOIN in breakdown.ts and the double-subquery in the export processor prop-keys query are clear improvements.

---

4. Security Concerns

No issues found. All game_id interpolations are numeric values from authenticated context. The parameterised {eventName: String} and {search: String} bindings are used correctly for user-supplied strings.

---

5. Test Coverage

The PR also fixes a pre-existing bug: two tests in included-data.test.ts (lines 40 and 69) were incorrectly inserting event-prop data into the events table instead of event_props. These were silent failures that are now corrected. The new test for live-player props with includeDevData = false is a welcome addition that completes the positive/negative coverage. event_props is also correctly added to the truncation list in setupTest.ts, preventing cross-test contamination.

---

Summary

Solid, well-scoped change. The only actionable item is propagating the dev_build guard into the breakdown subquery for consistency and a small performance benefit — everything else looks good.