Skip to content

Improve update polling for assessments#8102

Merged
lucanovera merged 2 commits into
mainfrom
ENG-3531-Improve-polling-after-assessment-is-triggered
May 4, 2026
Merged

Improve update polling for assessments#8102
lucanovera merged 2 commits into
mainfrom
ENG-3531-Improve-polling-after-assessment-is-triggered

Conversation

@lucanovera
Copy link
Copy Markdown
Contributor

@lucanovera lucanovera commented May 4, 2026

Ticket ENG-3531

Description Of Changes

Follow-up to #8041. With realistic LLM call durations (a few minutes per declaration), the privacy assessments list still didn't update until the first LLM call finished — so users could wait 3+ minutes after clicking "Generate" before any cards appeared, even though the materialized generating rows existed in the DB within ~2 seconds of trigger.

The original implementation looked for chang

This PR replaces both refetch triggers with a single polling subscription on the assessments list query, gated on the same !activeTask condition as the existing tasks polling. The page's useGetPrivacyAssessmentsQuery() shares the RTK cache key, so the list re-renders automatically when the indicator's polling fills the cache. Time-to-first-card drops from ~3 min to ~15s.

Code Changes

  • clients/admin-ui/src/features/privacy-assessments/AssessmentTaskStatusIndicator.tsx:
    • Added a useGetPrivacyAssessmentsQuery(undefined, { pollingInterval: 15s, skip: !activeTask }) subscription that mirrors the existing tasks-polling cadence.
    • Removed the prevCompletedCountRef effect (count-delta refetch trigger).
    • Removed the prevActiveTaskIdRef / hasObservedActiveTaskRef effect (one-shot refetch on task start).
    • Removed the now-unused completedCount derivation.
    • Kept the hadActiveTaskRef (active → idle) effect with its onTaskFinish?.() call: the polling subscription stops the moment activeTask becomes null, and the task's transition to complete happens in the same Celery commit as the last row's status flip. Without an explicit final refetch, ~50% of runs would leave one card showing stale state. The active → idle effect plugs that hole.
  • No changes to pages/privacy-assessments/index.tsx. Its onTaskFinish={refetchAssessments} wiring is what the active → idle effect calls — kept as-is.

Steps to Confirm

  1. Bring up the local stack: backend (fides + fidesplus) + admin-ui dev server.
  2. Trigger generation for a system that produces multiple declarations. Use a slow LLM (or stub time.sleep(30) in the LLM path) so each declaration takes well over 15s.
  3. Materialization gap: within ~15s of clicking "Generate assessments", confirm the Generating... cards appear in the list (no manual refresh needed).
  4. Per-row flips: as each LLM finishes, confirm the corresponding card flips to "Completeness X% / Resume" within ~15s, again without user interaction.
  5. Final state: when the task finishes, confirm the last card's state is correct (not stuck mid-flip) and the success toast fires.
  6. Failure path: trigger generation with an invalid LLM model. Confirm the error toast fires and any orphan generating rows are cleared (handled by the fidesplus side from Improve progress feedback on assessment evaluation #8041).
  7. Idle state: with no active task, navigate to /privacy-assessments. Network tab should show the initial list fetch only — no further polling while idle (skip: !activeTask keeps the subscription dormant).
  8. Mid-task page mount: refresh the page while a task is in flight. Cards should appear on the initial load and continue updating every ~15s until the task finishes.

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented May 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview May 4, 2026 8:50pm
fides-privacy-center Ignored Ignored May 4, 2026 8:50pm

Request Review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 4, 2026

Title Lines Statements Branches Functions
admin-ui Coverage: 8%
6.57% (2975/45259) 5.86% (1525/26007) 4.64% (620/13349)
fides-js Coverage: 78%
79.43% (2013/2534) 66.13% (1244/1881) 73.09% (345/472)
privacy-center Coverage: 88%
85.97% (331/385) 81.36% (179/220) 78.87% (56/71)

@lucanovera lucanovera marked this pull request as ready for review May 4, 2026 21:07
@lucanovera lucanovera requested a review from a team as a code owner May 4, 2026 21:07
@lucanovera lucanovera requested review from galvana and speaker-ender and removed request for a team and speaker-ender May 4, 2026 21:07
@lucanovera
Copy link
Copy Markdown
Contributor Author

@galvana I made this PR in response to your earlier feedback about needing a refresh of the page to see the results. This simplifies the logic and instead of looking at counts, it just refreshes the list results every 15s if there is an active task running.

@lucanovera lucanovera added this pull request to the merge queue May 4, 2026
Merged via the queue into main with commit 04cfb99 May 4, 2026
51 checks passed
@lucanovera lucanovera deleted the ENG-3531-Improve-polling-after-assessment-is-triggered branch May 4, 2026 21:19
galvana pushed a commit that referenced this pull request May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants