Skip to content

Conversation

aliu39
Copy link
Member

@aliu39 aliu39 commented Sep 10, 2025

Right now there's a chance we'll send additional start summary POST requests after the first finishes. Only one of these should be made per page load (segment count won't update until hard reload).

closes REPLAY-670: frontend should prevent infinite polling

Updates to polling timeout to useTimeout and simplifies shouldPoll (removing dependency on startSummaryRequestPending)

@aliu39 aliu39 requested a review from a team as a code owner September 10, 2025 00:55
@github-actions github-actions bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Sep 10, 2025
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@ryan953
Copy link
Member

ryan953 commented Sep 10, 2025

Lets take a look at this file together.

  • The useMutation can probably be replaced with like useApiQuery which does allow us to pass in a method and data. The benefit is that we can pass enabled as well and should be able to skip the useEffect that triggers it only once.
  • I also expected to see enabled passed into the useApiQuery call that returns summaryData.
  • I like how refetchInterval is used now
  • We can probably golf this down to be even more clear as to what the purpose is, but it's def getting there!

Copy link

linear bot commented Sep 15, 2025

Copy link

codecov bot commented Sep 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@             Coverage Diff             @@
##           master   #99182       +/-   ##
===========================================
+ Coverage   66.63%   81.23%   +14.60%     
===========================================
  Files        8575     8551       -24     
  Lines      379872   377273     -2599     
  Branches    24125    23991      -134     
===========================================
+ Hits       253120   306491    +53371     
+ Misses     126390    70427    -55963     
+ Partials      362      355        -7     


// Auto-start logic.
// TODO: remove the condition segmentCount <= 100
// when BE is able to process more than 100 segments. Without this, generation will loop.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need 100 segment condition anymore because the ref guarantees we only auto-POST once per page load

@aliu39 aliu39 changed the title fix(replay): prevent dup initial summary requests fix(replay): prevent dup initial summary requests and ref to use usetimeout Sep 15, 2025
cursor[bot]

This comment was marked as outdated.

@aliu39 aliu39 enabled auto-merge (squash) September 15, 2025 23:21
@aliu39 aliu39 merged commit e80eb8a into master Sep 15, 2025
47 checks passed
@aliu39 aliu39 deleted the aliu/summ-timeout branch September 15, 2025 23:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Frontend Automatically applied to PRs that change frontend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants