perf(relay): skip aggregate serialization on cheaply-decided updates

[Adamulek123]

Summary

shouldUpdateLiveActivity serialized both aggregates to JSON before checking the cheap field walks, paying two full serializations on every update path.

What changed

• Check activeCount and terminal transitions first; serialize only when they cannot decide. Ordering is behavior-identical (verified case-by-case in the diff): identical aggregates, attention included, are still suppressed and throttled exactly as before. Deliberately not changed: the health-check double stringify (already ordered cheapest-last; hashing would be a protocol change) and log-pipeline redaction (literal search is already optimal; regex would add overhead).

Validation

• vp test run infra/relay/src/agentActivity/ApnsDeliveries.test.ts — 50/50 pass.
• Package typecheck clean.

Summary by CodeRabbit

• Performance
  • Improved live activity update evaluation by checking common conditions earlier, reducing unnecessary processing.
• Bug Fixes
  • Ensured thread completion updates are not skipped due to throttling.
  • Preserved suppression of updates when aggregate data is unchanged.

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

⚙️ Run configuration

Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: efcb3a5d-9aea-4aea-8ea2-82fead335a29

📥 Commits

Reviewing files that changed from the base of the PR and between 3072e4e28faab52f188b95c6b9b6d214fc4683ae and 8014a19.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: c8e9165d-0479-4b0e-ad75-ab80c7c839ca

📥 Commits

Reviewing files that changed from the base of the PR and between 0f5a151 and 3072e4e28faab52f188b95c6b9b6d214fc4683ae.

📒 Files selected for processing (1)

• infra/relay/src/agentActivity/ApnsDeliveries.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.

---

📝 Walkthrough

Walkthrough

Changes

Live activity update evaluation

Layer / File(s)	Summary
Reorder update predicate checks infra/relay/src/agentActivity/ApnsDeliveries.ts	shouldUpdateLiveActivity checks activeCount and newly terminal rows before JSON serialization. Aggregate equality still suppresses identical updates before attention checks. Final outcomes remain unchanged.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Refactor

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 3072e

The optimization preserves live activity update decisions, with no remaining merge-blocking risk identified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 1 functions across 1 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Description check	✅ Passed	The description clearly explains the optimization, its rationale, preserved behavior, and validation results. It omits the template headings for Why, UI Changes, and Checklist, but the required inform…
Title check	✅ Passed	The title clearly and concisely describes the primary performance change: skipping aggregate serialization when cheaper checks decide the update.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

[macroscopeapp]

Approvability

Verdict: Approved at 3072e4e

Macroscope's review found this PR approvable — This is an isolated relay performance optimization that reorders existing checks to avoid unnecessary aggregate serialization. The live-activity delivery decisions and downstream behavior remain effectively unchanged, with no product-default, schema, security, or infrastructure impact.

No code changes detected at 8014a19. Prior analysis still applies.

^{You can add or adjust custom eligibility rules. Learn more.}

[Adamulek123]

Reviving this one after a staleness re-check against current main (ec80933ac8cd).

Everything about this one still holds up:

• The base blob for infra/relay/src/agentActivity/ApnsDeliveries.ts is byte-identical to current main (651f031f0efa) — the file hasn't been touched since feat(mobile): add Android agent notifications and ongoing activity #10416 on 2026-09-09, so this diff needs no rebase and applies exactly as written.
• shouldUpdateLiveActivity (ApnsDeliveries.ts:163-197) is in the same shape the diff expects.
• mergeable: MERGEABLE, mergeStateStatus: CLEAN.
• Macroscope returned a real APPROVED on 2026-09-16 ("Approved at 3072e4e") — the only genuine approval in this cluster.

I also re-verified the reorder is behavior-identical, since that's the whole risk in a predicate reshuffle. Moving activeCount and newlyTerminalRows above the JSON-equality guard is safe in both cases:

• A differing activeCount (:175) implies the serialized aggregates differ, so it would have failed the equality guard anyway.
• newlyTerminalRows(prev, next, true).length > 0 (agentActivityAlerts.ts:72-88) requires a row completed/failed in next but not in prev's phase map. Identical aggregates means identical phases, so the set is necessarily empty.

And the checks moved below the guard keep their original relative order, so the "identical aggregates with attention rows are still suppressed" case is preserved. The explanatory comment at :181-183 about terminal rows bypassing throttling survives intact in its new position.

One gap I want to close before this goes back into a review queue: the PR body currently cites a passing test count and "behavior-identical (verified case-by-case in the diff)", but CONTRIBUTING asks for evidence matched to the change, and this is a perf claim. Could you add a measured before/after to the body — even a rough microbenchmark on shouldUpdateLiveActivity with the row counts you care about. That is the only thing standing between this and a clean approval as far as I can tell.

For context on why this is worth a maintainer's attention now: I triaged nine long-stuck PRs from this queue and withdrew four. This is the smallest and cleanest of what remained — independent file, no behavioral change, no product-default or schema impact, already approved once. Happy to rebase or re-run anything if that's easier than editing the body.

[maria-rcks]

Note

Written by claude-opus-5-5 on behalf of Maria

Hi! We are cleaning up open PRs, and this one does not say which model created it and was opened more than two weeks ago. If this change is still important, please rebuild it on current main with a newer model and note the model in the PR description.