Skip to content

fix(activity-sidebar): remove UAA parity logging#4719

Merged
mergify[bot] merged 1 commit into
box:masterfrom
kduncanhsu:kduncanhsu/remove-uaa-parity-logging
Jul 23, 2026
Merged

fix(activity-sidebar): remove UAA parity logging#4719
mergify[bot] merged 1 commit into
box:masterfrom
kduncanhsu:kduncanhsu/remove-uaa-parity-logging

Conversation

@kduncanhsu

@kduncanhsu kduncanhsu commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Remove the compareV2AndUaaFeedItems parity check that fired the full legacy v2 API fan-out (comments, annotations, tasks, versions, app activity) on every preview load when UAA was enabled, solely for comparison logging
  • Guard legacy API calls with !shouldUseUAA so they are never fired when the UAA /file_activities endpoint is in use
  • Remove the METRIC_TYPE_UAA_PARITY_METRIC constant and associated logging infrastructure (logAPIParity, Logger special case)

Context

When activityFeed.uaaIntegration.enabled is on, the sidebar renders from the UAA response but still fired ~6 legacy API calls per preview load for parity validation. This added ~12.6M getComments calls/day to the annotations service (~2x its total volume). Parity has been validated and is no longer needed.

The original PR which added UAA parity logging is: #3629

Test plan

  • yarn test src/api/__tests__/Feed.test.js — 138 tests pass
  • yarn test src/elements/content-sidebar/__tests__/ActivitySidebar.test.js — 133 tests pass
  • yarn test src/elements/common/logger/__tests__/Logger.test.js — 10 tests pass
  • yarn flow check passes on modified files
  • Verify in preview that activity sidebar loads correctly with UAA enabled
  • Confirm annotations service traffic drops after deploy

Summary by CodeRabbit

  • New Features
    • Enhanced file activity feed processing for improved UAA-based activity handling.
    • Added clearer initial status for newly created pending task feed items.
  • Bug Fixes
    • Prevented unnecessary legacy V2 feed requests when the enhanced/UAA flow is enabled.
  • Refactor
    • Removed legacy V2 vs UAA parity comparison and related parity logging.
  • Tests
    • Updated feed and activity sidebar tests to match the new request and metric behavior.
  • Chores
    • Simplified preview/activity metric types and standardized preview metric emission.

@kduncanhsu
kduncanhsu requested review from a team as code owners July 22, 2026 23:33
@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

The feed flow now uses UAA file activities directly when enabled, skips V2 feed API calls, removes parity logging, updates sidebar feature-flag wiring, and replaces the removed parity metric with explicit metric constants and types.

Changes

UAA Feed Flow

Layer / File(s) Summary
Direct UAA feed orchestration
src/api/Feed.js, src/api/__tests__/Feed.test.js
UAA mode skips V2 activity fetches, parses UAA file activities directly, and verifies that V2 APIs are not called.
Sidebar feed API wiring
src/elements/content-sidebar/ActivitySidebar.js, src/elements/content-sidebar/__tests__/ActivitySidebar.test.js
The sidebar passes explicit feature flags to feedItems, removes the parity callback, and updates call-shape expectations.
Metric contract cleanup
src/constants.js, src/common/types/logging.js, src/elements/common/logger/Logger.js
UAA parity metrics are removed; preview, elements performance, and elements load metric types are defined and preview logging always emits the preview type.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ActivitySidebar
  participant FeedAPI
  participant UAAFileActivities
  ActivitySidebar->>FeedAPI: call feedItems with UAA flags
  FeedAPI->>UAAFileActivities: fetch file activities
  UAAFileActivities-->>FeedAPI: return activities
  FeedAPI->>FeedAPI: parse and handle feed items
Loading

Possibly related PRs

Suggested labels: ready-to-merge

Suggested reviewers: ahorowitz123, zhirongwang, jackiejou

Poem

I’m a rabbit with a tidy feed,
UAA supplies the facts I need.
V2 calls hop away,
Parity metrics fade,
Clean flags guide the burrow today. 🐇

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately and concisely summarizes the main change: removing UAA parity logging from the activity sidebar.
Description check ✅ Passed The description includes the required summary, context, and test plan, and it clearly explains the UAA parity logging removal.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@kduncanhsu
kduncanhsu force-pushed the kduncanhsu/remove-uaa-parity-logging branch from 1596889 to 6a95178 Compare July 23, 2026 21:14

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
src/api/Feed.js (1)

614-627: 🚀 Performance & Scalability | 🔵 Trivial

Verify the traffic reduction before rollout.

Confirm in preview and post-deployment metrics that legacy annotation, comment, task, app-activity, and version requests remain absent for UAA-enabled loads, while file_activities success rates remain healthy.

As per PR objectives, preview validation and post-deployment traffic verification remain pending.

🤖 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 `@src/api/Feed.js` around lines 614 - 627, Validate the UAA request gating
around annotationsPromise, commentsPromise, tasksPromise, appActivityPromise,
versionsPromise, and currentVersionPromise in preview and after deployment:
confirm these legacy requests are absent for UAA-enabled loads, and verify
file_activities success rates remain healthy before completing rollout.
🤖 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.

Nitpick comments:
In `@src/api/Feed.js`:
- Around line 614-627: Validate the UAA request gating around
annotationsPromise, commentsPromise, tasksPromise, appActivityPromise,
versionsPromise, and currentVersionPromise in preview and after deployment:
confirm these legacy requests are absent for UAA-enabled loads, and verify
file_activities success rates remain healthy before completing rollout.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: af7cc76a-a6fb-452a-b983-1a8628d07132

📥 Commits

Reviewing files that changed from the base of the PR and between 1596889 and 6a95178.

📒 Files selected for processing (7)
  • src/api/Feed.js
  • src/api/__tests__/Feed.test.js
  • src/common/types/logging.js
  • src/constants.js
  • src/elements/common/logger/Logger.js
  • src/elements/content-sidebar/ActivitySidebar.js
  • src/elements/content-sidebar/__tests__/ActivitySidebar.test.js
💤 Files with no reviewable changes (3)
  • src/constants.js
  • src/elements/content-sidebar/ActivitySidebar.js
  • src/elements/content-sidebar/tests/ActivitySidebar.test.js
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/elements/common/logger/Logger.js

@mergify

mergify Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-07-23 23:45 UTC · Rule: Automatic strict merge · triggered by rule Automatic merge queue
  • Checks skipped · PR is already up-to-date
  • Merged2026-07-23 23:45 UTC · at 6a9517809a96a2fac744bfe105f82f408182cdd4 · squash

This pull request spent 11 seconds in the queue, including 1 second running CI.

Required conditions to merge
  • github-review-approved [🛡 GitHub branch protection] (documentation)
  • github-review-decision = APPROVED [🛡 GitHub branch protection] (documentation)
  • any of [🛡 GitHub branch protection]:
    • check-success = Summary
    • check-neutral = Summary
    • check-skipped = Summary
  • any of [🛡 GitHub branch protection]:
    • check-success = lint_test_build
    • check-neutral = lint_test_build
    • check-skipped = lint_test_build
  • any of [🛡 GitHub branch protection]:
    • check-success = license/cla
    • check-neutral = license/cla
    • check-skipped = license/cla
  • any of [🛡 GitHub branch protection]:
    • check-success = lint_pull_request
    • check-neutral = lint_pull_request
    • check-skipped = lint_pull_request

@mergify
mergify Bot merged commit dce056d into box:master Jul 23, 2026
12 of 13 checks passed
@mergify mergify Bot removed the queued label Jul 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants