fix(dashboard): catch up GitHub signal sync with cached query age#161
fix(dashboard): catch up GitHub signal sync with cached query age#161
Conversation
Compare server revalidation timestamps to React Query dataUpdatedAt on first sync and on WebSocket reconnect so offline webhook invalidations refetch lists.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 7 minutes and 57 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a shared Changes
Sequence Diagram(s)sequenceDiagram
participant Hook as useGitHubSignalStream
participant WS as WebSocket
participant API as Server (getRevalidationSignalTimestamps)
participant Poll as PollingTimer
participant Invalidator as invalidateTargets
Hook->>WS: open connection / subscribe
WS->>API: request revalidation timestamps (on open)
API-->>WS: [{signalKey, updatedAt}, ...]
WS->>Hook: deliver server timestamps
Hook->>Hook: collectKeysToInvalidateAfterServerSync(lastSeenTimestampsRef, serverTs, localDataUpdatedAt)
Hook->>Invalidator: invalidate computed signalKeys
Poll->>Hook: periodic poll reads lastSeenTimestampsRef
Hook->>Hook: collectKeysToInvalidateAfterServerSync(...)
Hook->>Invalidator: invalidate computed signalKeys
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/lib/use-github-signal-stream.ts`:
- Around line 95-115: The current lastSeenTimestamps map keyed only by
signal.signalKey causes per-target first-sync checks to be skipped once any
target sets a lastSeen; update the logic to track first-sync state per target by
using a composite key (e.g. `${target.queryKey}:${signal.signalKey}`) or reset
entries when the targets/query set changes; modify usages around
lastSeenTimestamps, the loop over targets, and the branch that sets
lastSeenTimestamps (and replicate the same fix at the other occurrence around
lines 381-383) so the comparison against
queryClient.getQueryState(target.queryKey)?.dataUpdatedAt is performed per
target rather than globally per signalKey.
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 22e6b674-c07d-48bb-81ed-3c01671ed3df
📒 Files selected for processing (1)
apps/dashboard/src/lib/use-github-signal-stream.ts
Resolve use-github-signal-stream: keep mergedTargets for subscriptions and wire lastSeenTimestampsRef through poll/WebSocket catch-up.
Key lastSeen by queryKey+signalKey so new targets sharing pullsMine still get catch-up vs dataUpdatedAt; reset baseline when merged target set changes.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/dashboard/src/lib/use-github-signal-stream.ts (1)
145-167:⚠️ Potential issue | 🟠 MajorTrack first-sync state per target, not just per signal key.
Line 146 keys
lastSeenTimestampsonly bysignalKey, and Lines 438-440 only clear that map when the subscribed key set changes. IfmergedTargetschanges but reuses the same signal key, the new target skips thedataUpdatedAtcatch-up path and can keep stale persisted data until the next GitHub update. Key this state byqueryKey + signalKey, or reset it when the target set changes too.Also applies to: 438-440
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/use-github-signal-stream.ts` around lines 145 - 167, The bug is that lastSeenTimestamps is keyed only by signal.signalKey so when mergedTargets (or targets) changes but reuses the same signalKey a new target won’t run the catch-up logic; update the usage of lastSeenTimestamps to key by a composite key (e.g., `${queryKey}:${signalKey}`) wherever you call get/set/has/clear in this file (including inside the signals loop and the place that currently clears the map when subscribed keys change), or alternatively clear/reset lastSeenTimestamps whenever mergedTargets/targets change so each target will trigger the dataUpdatedAt catch-up; adjust references to lastSeenTimestamps.get, .set and the clearing logic accordingly (search for lastSeenTimestamps and mergedTargets to find all update points).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/dashboard/src/lib/use-github-signal-stream.ts`:
- Line 2: Replace the deprecated MutableRefObject import with RefObject in the
module import statement and update any type annotations that currently use
MutableRefObject<Map<string, number>> to RefObject<Map<string, number>> (notably
the ref declarations used inside the useGithubSignalStream hook); ensure the
hook and any functions that mutate the ref continue to access and assign via
.current so the unified RefObject type works correctly with React 19.
---
Duplicate comments:
In `@apps/dashboard/src/lib/use-github-signal-stream.ts`:
- Around line 145-167: The bug is that lastSeenTimestamps is keyed only by
signal.signalKey so when mergedTargets (or targets) changes but reuses the same
signalKey a new target won’t run the catch-up logic; update the usage of
lastSeenTimestamps to key by a composite key (e.g., `${queryKey}:${signalKey}`)
wherever you call get/set/has/clear in this file (including inside the signals
loop and the place that currently clears the map when subscribed keys change),
or alternatively clear/reset lastSeenTimestamps whenever mergedTargets/targets
change so each target will trigger the dataUpdatedAt catch-up; adjust references
to lastSeenTimestamps.get, .set and the clearing logic accordingly (search for
lastSeenTimestamps and mergedTargets to find all update points).
🪄 Autofix (Beta)
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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0ea8bdc9-619c-418f-9983-594a98395bac
📒 Files selected for processing (1)
apps/dashboard/src/lib/use-github-signal-stream.ts
| @@ -1,5 +1,5 @@ | |||
| import { type QueryKey, useQueryClient } from "@tanstack/react-query"; | |||
| import { useEffect, useMemo, useRef } from "react"; | |||
| import { type MutableRefObject, useEffect, useMemo, useRef } from "react"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Declared React versions in package manifests:"
python - <<'PY'
import json
from pathlib import Path
for path in Path(".").rglob("package.json"):
try:
data = json.loads(path.read_text())
except Exception:
continue
deps = {}
for bucket in ("dependencies", "devDependencies", "peerDependencies"):
deps.update(data.get(bucket, {}))
react = deps.get("react")
types = deps.get("@types/react")
if react or types:
print(f"{path}: react={react or 'n/a'}, `@types/react`={types or 'n/a'}")
PY
echo
echo "Current MutableRefObject usages:"
rg -n --type=ts --type=tsx '\bMutableRefObject\b'Repository: stylessh/diffkit
Length of output: 376
🏁 Script executed:
cat -n apps/dashboard/src/lib/use-github-signal-stream.ts | head -200Repository: stylessh/diffkit
Length of output: 6750
🏁 Script executed:
rg -n 'MutableRefObject' --type ts apps/dashboard/src/lib/Repository: stylessh/diffkit
Length of output: 423
🏁 Script executed:
cat -n apps/dashboard/src/lib/use-github-signal-stream.ts | sed -n '170,200p; 330,360p'Repository: stylessh/diffkit
Length of output: 1909
🌐 Web query:
React 19 MutableRefObject RefObject ref type changes
💡 Result:
In React 19, significant changes were made to ref types in TypeScript via @types/react updates to align with runtime improvements, particularly making refs mutable by default and simplifying useRef. Key changes: - useRef now requires an initial argument (e.g., useRef(null) or useRef(undefined)). The zero-argument form is removed. - RefObject now has mutable current: T (not readonly T | null). Previously, current was readonly T | null. - MutableRefObject is deprecated in favor of the unified RefObject, which useRef always returns. - For ref as a prop (new React 19 feature, reducing need for forwardRef), use React.Ref for the prop type (union of RefObject | ((instance: T | null) => void) | null | undefined). These changes improve type safety and reflect common usage patterns. Codemods like types-react-codemod refobject-defaults and useref-required-initial help migration. Examples: const ref = useRef(null); // RefObject, current: HTMLDivElement | null (but mutable) ref.current = someElement; // Now allowed, previously readonly error // For component props in React 19: interface Props { ref?: React.Ref; } function MyComponent({ ref, ...props }: Props) { return <div ref={ref} {...props} />; } Official React docs do not detail TypeScript types (runtime-focused), but upgrade guide mentions ref cleanups and useRef requiring argument. Changes implemented in @types/react for React 19 compatibility.
Citations:
- 1: https://react.dev/blog/2024/04/25/react-19-upgrade-guide
- 2: https://beta.reactjs.org/blog/2024/04/25/react-19-upgrade-guide
- 3: [react] Types for React 19 DefinitelyTyped/DefinitelyTyped#69022
- 4: [react] Make all refs mutable by default DefinitelyTyped/DefinitelyTyped#64896
Replace deprecated MutableRefObject with RefObject for React 19 compatibility.
MutableRefObject is deprecated in React 19 as of @types/react@^19.2.0. Refactor line 2's import and the type annotations on lines 187 and 349 to use RefObject<Map<string, number>> instead, which is the unified ref type in React 19 with mutable .current.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/dashboard/src/lib/use-github-signal-stream.ts` at line 2, Replace the
deprecated MutableRefObject import with RefObject in the module import statement
and update any type annotations that currently use MutableRefObject<Map<string,
number>> to RefObject<Map<string, number>> (notably the ref declarations used
inside the useGithubSignalStream hook); ensure the hook and any functions that
mutate the ref continue to access and assign via .current so the unified
RefObject type works correctly with React 19.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
apps/dashboard/src/lib/use-github-signal-stream.ts (1)
2-2:⚠️ Potential issue | 🟠 MajorReplace
MutableRefObjectwithRefObjectfor React 19 typings.Line 2, Line 187, and Line 349 still use
MutableRefObject. With React 19 typings, this should be migrated toRefObjectto avoid deprecation/compatibility drift.Proposed diff
-import { type MutableRefObject, useEffect, useMemo, useRef } from "react"; +import { type RefObject, useEffect, useMemo, useRef } from "react"; ... function useGitHubSignalStreamWebSocket( targets: readonly GitHubSignalStreamTarget[], signalKeysKey: string, - lastSeenTimestampsRef: MutableRefObject<Map<string, number>>, + lastSeenTimestampsRef: RefObject<Map<string, number>>, ) { ... function useGitHubSignalPoll( targets: readonly GitHubSignalStreamTarget[], signalKeysKey: string, - lastSeenTimestampsRef: MutableRefObject<Map<string, number>>, + lastSeenTimestampsRef: RefObject<Map<string, number>>, ) {Use this read-only check to verify package typings + remaining usages:
#!/bin/bash set -euo pipefail python - <<'PY' import json from pathlib import Path for path in Path(".").rglob("package.json"): try: data = json.loads(path.read_text()) except Exception: continue deps = {} for bucket in ("dependencies", "devDependencies", "peerDependencies"): deps.update(data.get(bucket, {})) if "react" in deps or "@types/react" in deps: print(f"{path}: react={deps.get('react')}, `@types/react`={deps.get('@types/react')}") PY echo echo "MutableRefObject usages:" rg -n --type=ts --type=tsx '\bMutableRefObject\b'Also applies to: 187-187, 349-349
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/dashboard/src/lib/use-github-signal-stream.ts` at line 2, The code imports and uses MutableRefObject which is deprecated in React 19 typings; replace MutableRefObject with RefObject in the import from react and in all type annotations inside this module (e.g., the import line and any variables/parameters typed as MutableRefObject<T> around the useGitHubSignalStream hook and related refs), changing signatures like MutableRefObject<...> to RefObject<...> so the types align with React 19; ensure you import RefObject from "react" and update any places that assume mutability to use a currentRef access pattern (ref.current) if mutation is needed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/dashboard/src/lib/use-github-signal-stream.ts`:
- Line 2: The code imports and uses MutableRefObject which is deprecated in
React 19 typings; replace MutableRefObject with RefObject in the import from
react and in all type annotations inside this module (e.g., the import line and
any variables/parameters typed as MutableRefObject<T> around the
useGitHubSignalStream hook and related refs), changing signatures like
MutableRefObject<...> to RefObject<...> so the types align with React 19; ensure
you import RefObject from "react" and update any places that assume mutability
to use a currentRef access pattern (ref.current) if mutation is needed.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1fa9e9da-b7cf-452d-a72e-866cced0a51e
📒 Files selected for processing (1)
apps/dashboard/src/lib/use-github-signal-stream.ts
1 participant
Summary
Fixes stale "My" / overview PR and issue lists after webhooks ran while the user was away. The signal poll previously seeded server timestamps on first sync without comparing them to React Query cache age, so durable D1 invalidations never triggered a refetch. WebSocket reconnect now runs the same catch-up immediately.
Changes
github_revalidation_signaltimestamps todataUpdatedAton first sync per keylastSeenmap between poll and WS; reset when subscribed signal keys changeTest Plan
Screenshots
N/A (behavioral fix; no visual design change)
Summary by CodeRabbit
Bug Fixes
Refactor