fix: reject stale child-market alerts - #13
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe activity pipeline now uses validated Polymarket market condition IDs instead of event IDs. Preview normalization, market selection, API fetching, signal generation, notice ingestion, polling, rendering, fixtures, and tests apply current-market and condition-ID validation. ChangesMarket-Condition Activity
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ActivityRail
participant MarketSelector
participant GlobalConflictActivity
participant PolymarketActivityAPI
ActivityRail->>MarketSelector: Select current market IDs
ActivityRail->>GlobalConflictActivity: Request activity with marketIds
GlobalConflictActivity->>PolymarketActivityAPI: Fetch market activity
PolymarketActivityAPI-->>GlobalConflictActivity: Return trades with conditionId
GlobalConflictActivity-->>ActivityRail: Return normalized activity
ActivityRail->>ActivityRail: Validate current event and market condition
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
tests/global-conflict-map-preview.spec.ts (1)
21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider one shared test helper for condition IDs.
mockConditionIdduplicatesconditionIdintests/polymarket-activity-query.spec.ts(Lines 8-10) and the inline0x${...padStart(64, "0")}expressions intests/conflict-activity-signals.spec.ts(Lines 16, 78, 116). One shared helper keeps the fixture format aligned with the production pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/global-conflict-map-preview.spec.ts` around lines 21 - 23, Create one shared test helper for generating condition IDs, then replace mockConditionId in global-conflict-map-preview.spec.ts, conditionId in polymarket-activity-query.spec.ts, and the matching inline expressions in conflict-activity-signals.spec.ts with that helper. Preserve the existing hexadecimal, 64-character zero-padded format.features/global-conflict-map/preview/activity-rail.tsx (1)
364-383: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low value
currentActivityEventsis time-dependent but memoized only onfeed.events.
isPolymarketActivityEventCurrentreadsDate.now()internally, and this memo only recomputes whenfeed.eventschanges. Between feed refreshes an event that passes itsendDatestays incurrentActivityEventIdsandcurrentActivityMarketConditionIds, so it keeps qualifying for notice ingestion and for themarketIdQuery. The rendered output stays correct because the visible filter at Lines 546-561 re-checks againstclock. Consider addingclockas a dependency so ingestion and polling age out with the same cadence as rendering.♻️ Proposed change
const currentActivityEvents = useMemo( () => feed.events.filter((event) => - isPolymarketActivityEventCurrent(event), + isPolymarketActivityEventCurrent(event, clock), ), - [feed.events], + [clock, feed.events], );Note that
clockticks every 5 seconds, so this makes the dependent memos and the polling effect re-run on that cadence. Confirm the added churn is acceptable before applying.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@features/global-conflict-map/preview/activity-rail.tsx` around lines 364 - 383, Update the currentActivityEvents useMemo dependency list to include clock alongside feed.events, so isPolymarketActivityEventCurrent is re-evaluated on each clock tick. Keep currentActivityEventIds and currentActivityMarketConditionIds derived from the refreshed event set, preserving the existing polling and ingestion behavior.lib/conflict-activity-signals.ts (1)
131-137: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe volume check here is now redundant.
currentEventsalready enforcesevent.volume >= POLYMARKET_ACTIVITY_EVENT_MIN_VOLUMEthroughisPolymarketActivityEventCurrent. The same duplicate check exists inrankedOddsSignalat Line 49. Removing them keeps the threshold defined in one place.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/conflict-activity-signals.ts` around lines 131 - 137, Remove the redundant event.volume threshold predicate from the volumeLeaders filter in the conflict activity signal, relying on currentEvents and isPolymarketActivityEventCurrent to enforce it. Apply the same removal in rankedOddsSignal, while preserving the volume24h and selectedEventIds filters.app/api/global-conflict-activity/route.ts (1)
197-211: 🩺 Stability & Availability | 🔵 TrivialWatch the cache and single-flight key cardinality after this change.
The key is now the exact set of market condition IDs. Previously it was the event-ID set. Clients that load the feed at different refresh boundaries can produce slightly different ID sets, and each distinct set creates a separate
unstable_cacheentry and a separate upstream fetch. Add a metric on the distinct-key count and the upstream request rate so a drop in cache hit ratio is visible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@app/api/global-conflict-activity/route.ts` around lines 197 - 211, Instrument getSingleFlightActivityFeed to measure distinct market-ID cache keys and upstream request rate. Record a metric when a new key is created in activityRefreshesInFlight and another when getCachedActivityFeed triggers an upstream request, using the project’s existing metrics mechanism and preserving the current single-flight behavior.tests/polymarket-activity-query.spec.ts (1)
12-54: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider covering the 100-market cap and the exclusion of the non-Polymarket event.
The fixture yields 72 eligible markets, so
POLYMARKET_ACTIVITY_MAX_MARKET_IDSand the volume-based truncation are never exercised. Thefixture-eventcase is also only checked indirectly through the length assertion.♻️ Suggested additions
expect(marketIds).not.toContain(conditionId(997)); expect(marketIds).not.toContain(conditionId(999)); + expect(marketIds).not.toContain(conditionId(1_000)); }); + +test("caps selection at the maximum market count and keeps the highest volumes", () => { + const now = Date.parse("2026-08-11T13:00:00Z"); + const events = Array.from({ length: 130 }, (_, index) => ({ + id: `polymarket-${710_000 + index}`, + volume: 400_000 + index, + endDate: "2026-08-31T23:59:00Z", + marketConditionId: conditionId(index + 1), + })); + + const marketIds = selectPolymarketActivityMarketIds(events, now); + + expect(marketIds).toHaveLength(100); + expect(marketIds).toContain(conditionId(130)); + expect(marketIds).not.toContain(conditionId(1)); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/polymarket-activity-query.spec.ts` around lines 12 - 54, Extend the test around selectPolymarketActivityMarketIds to include more than POLYMARKET_ACTIVITY_MAX_MARKET_IDS eligible markets, verifying the result is capped and retains the highest-volume markets. Add an explicit assertion that fixture-event is excluded, while preserving the existing expired, missing-condition, and below-threshold exclusions.lib/polymarket-conflict-preview.ts (1)
147-151: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe condition-ID format is restated in seven places. The pattern
0xfollowed by 64 hexadecimal characters is defined independently in four production modules and rebuilt independently in three test files. A change to the format would require edits in all of them.
lib/polymarket-conflict-preview.ts#L147-L151: export one sharedCONDITION_ID_PATTERNconstant, and import it here, inlib/polymarket-activity-query.ts(Line 8), inapp/api/global-conflict-activity/route.ts(Line 32), and infeatures/global-conflict-map/preview/activity-rail.tsx(Line 46).tests/global-conflict-map-preview.spec.ts#L21-L23: movemockConditionIdinto a shared test helper module, and use it in place ofconditionIdintests/polymarket-activity-query.spec.ts(Lines 8-10) and the inline expressions intests/conflict-activity-signals.spec.ts(Lines 16, 78, 116).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lib/polymarket-conflict-preview.ts` around lines 147 - 151, Centralize the condition-ID format by exporting a shared CONDITION_ID_PATTERN from normalizeConditionId in lib/polymarket-conflict-preview.ts (147-151), then import and reuse it in lib/polymarket-activity-query.ts (8), app/api/global-conflict-activity/route.ts (32), and features/global-conflict-map/preview/activity-rail.tsx (46). Move mockConditionId from tests/global-conflict-map-preview.spec.ts (21-23) into a shared test helper, and reuse it in tests/polymarket-activity-query.spec.ts (8-10) and tests/conflict-activity-signals.spec.ts (16, 78, 116), removing the duplicated condition-ID expressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lib/polymarket-activity-query.ts`:
- Around line 17-33: Align endDate handling between market selection and
isPolymarketActivityEventCurrent: ensure markets with null endDate are either
consistently required to have a future endDate during selection or accepted by
the activity eligibility check. Preserve eligibility for otherwise valid markets
and use the existing normalization and selection symbols rather than introducing
separate semantics.
In `@tests/conflict-activity-signals.spec.ts`:
- Around line 62-85: Update the neighboring staleness test around
buildRollingActivitySignals so its event fixture includes a valid future endDate
and marketConditionId. Keep the snapshot old enough to exercise the maximumAge
freshness guard, ensuring the expected empty result is caused by staleness
rather than isPolymarketActivityEventCurrent rejecting null market metadata.
In `@tests/global-conflict-map-preview.spec.ts`:
- Around line 1493-1523: Update the mixedDeadlineEvent fixture and its expected
endDate in the test around normalizeConflictPreviewEvent to use dates computed
relative to Date.now(), following the existing Date.now() + 7 * 24 * 60 * 60_000
pattern. Keep the two markets ordered so the later deadline remains selected and
the assertion continues validating the August-market behavior without fixed
expired dates.
---
Nitpick comments:
In `@app/api/global-conflict-activity/route.ts`:
- Around line 197-211: Instrument getSingleFlightActivityFeed to measure
distinct market-ID cache keys and upstream request rate. Record a metric when a
new key is created in activityRefreshesInFlight and another when
getCachedActivityFeed triggers an upstream request, using the project’s existing
metrics mechanism and preserving the current single-flight behavior.
In `@features/global-conflict-map/preview/activity-rail.tsx`:
- Around line 364-383: Update the currentActivityEvents useMemo dependency list
to include clock alongside feed.events, so isPolymarketActivityEventCurrent is
re-evaluated on each clock tick. Keep currentActivityEventIds and
currentActivityMarketConditionIds derived from the refreshed event set,
preserving the existing polling and ingestion behavior.
In `@lib/conflict-activity-signals.ts`:
- Around line 131-137: Remove the redundant event.volume threshold predicate
from the volumeLeaders filter in the conflict activity signal, relying on
currentEvents and isPolymarketActivityEventCurrent to enforce it. Apply the same
removal in rankedOddsSignal, while preserving the volume24h and selectedEventIds
filters.
In `@lib/polymarket-conflict-preview.ts`:
- Around line 147-151: Centralize the condition-ID format by exporting a shared
CONDITION_ID_PATTERN from normalizeConditionId in
lib/polymarket-conflict-preview.ts (147-151), then import and reuse it in
lib/polymarket-activity-query.ts (8), app/api/global-conflict-activity/route.ts
(32), and features/global-conflict-map/preview/activity-rail.tsx (46). Move
mockConditionId from tests/global-conflict-map-preview.spec.ts (21-23) into a
shared test helper, and reuse it in tests/polymarket-activity-query.spec.ts
(8-10) and tests/conflict-activity-signals.spec.ts (16, 78, 116), removing the
duplicated condition-ID expressions.
In `@tests/global-conflict-map-preview.spec.ts`:
- Around line 21-23: Create one shared test helper for generating condition IDs,
then replace mockConditionId in global-conflict-map-preview.spec.ts, conditionId
in polymarket-activity-query.spec.ts, and the matching inline expressions in
conflict-activity-signals.spec.ts with that helper. Preserve the existing
hexadecimal, 64-character zero-padded format.
In `@tests/polymarket-activity-query.spec.ts`:
- Around line 12-54: Extend the test around selectPolymarketActivityMarketIds to
include more than POLYMARKET_ACTIVITY_MAX_MARKET_IDS eligible markets, verifying
the result is capped and retains the highest-volume markets. Add an explicit
assertion that fixture-event is excluded, while preserving the existing expired,
missing-condition, and below-threshold exclusions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a6ae791-03a2-44a0-8751-b45be89de249
📒 Files selected for processing (10)
app/api/global-conflict-activity/route.tsfeatures/global-conflict-map/preview/activity-rail.tsxfeatures/global-conflict-map/preview/fixture.tsfeatures/global-conflict-map/preview/types.tslib/conflict-activity-signals.tslib/polymarket-activity-query.tslib/polymarket-conflict-preview.tstests/conflict-activity-signals.spec.tstests/global-conflict-map-preview.spec.tstests/polymarket-activity-query.spec.ts
## Summary - bump the persistent conflict-feed cache key after adding `marketConditionId` to the normalized event schema - force Vercel Data Cache to rebuild live events instead of reusing pre-#13 objects ## Production finding After #13 deployed, `oddsfront.com` correctly failed closed but the public feed still returned a pre-deploy cached object without `marketConditionId`, leaving zero alert-eligible markets. The cache key version makes the new schema available immediately. ## Verification - clean `npm ci` — 0 vulnerabilities - focused ESLint — passed - `npm run typecheck` — passed - public probe identified the stale v3 cache before this bump
1 participant
Summary
Root cause
Polymarket event
707496contains multiple deadline markets. The aggregate event query returned a recent trade from the expired July 31 child even though the map had correctly selected the future August 31 child.Verification
npx playwright test tests/polymarket-activity-query.spec.ts tests/conflict-activity-signals.spec.ts— 7 passednpm run check— lint, typecheck, and production build passedYES %form, footerflex+nowrapSummary by CodeRabbit