Skip to content

perf(client-runtime): bucket workflow phase members once - #31199

Open
hieu-lee wants to merge 1 commit into
pingdotgg:mainfrom
hieu-lee:hieu-lee/bucket-workflow-phase-members
Open

hieu-lee wants to merge 1 commit into
pingdotgg:mainfrom
hieu-lee:hieu-lee/bucket-workflow-phase-members

Conversation

@hieu-lee

@hieu-lee hieu-lee commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Workflow grouping filters every member again for each phase. With P phases and M members, membership work is O(P × M), which becomes quadratic when both grow.

Solution

Bucket members by phase during the existing title-discovery pass, then sort and return those same buckets. Membership work becomes O(M); overall grouping is O(A log A + P log P) at worst, including existing sorts, with unchanged O(A + P) space. Declared/inferred titles, stable member ordering, status counts and unphased members keep the same behavior.

Validation (tests extended before production code):

  • vp test run packages/client-runtime/src/state/subagentRuntime.test.ts apps/mobile/src/features/threads/threadAgentsPresentation.test.ts: 41 tests passed.
  • Actual-source proof: 2,009 complete-output fixtures passed, including reference-identity and frozen-input checks. Operation-count scaling confirms the repeated phase/member scan is removed; sorting costs remain.
  • vp run check:changed: all selected checks passed.
  • Three independent Codex reviewers returned clean. This pure derivation change has no rendering, navigation or connection behavior change; no layout screenshot is claimed.

Related: #30390 improves titles in the same helper module, and #24055 changes web subscription behavior. Full overlapping diffs were checked; they do not implement phase bucketing. This is a focused behavior-preserving performance fix, not a new feature; no prior maintainer approval is claimed.

TLDR

complexity-workflow.mp4

Model: GPT-6.1 Sol. Harness: Codex in T3 Code.

@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 4, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 89a766e

Macroscope's review found this PR approvable — Workflow members are bucketed once by phase instead of repeatedly scanned. The change is confined to in-memory presentation derivation, preserving existing grouping behavior without introducing persistent or external effects.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 4, 2026

Copy link
Copy Markdown
Contributor

Explain Change →

No actionable comments were generated in the recent review.

Walkthrough

deriveWorkflowGroups now groups workflow members by phase index in one pass, then sorts phase and unphased members by agent index. Tests cover ordering, phase titles and states, settled counts, empty phases, and input immutability.

Changes: Workflow grouping

Layer / File(s) Summary
Group members by phase
packages/client-runtime/src/state/subagentRuntime.ts, packages/client-runtime/src/state/subagentRuntime.test.ts
The function groups members by phase index, including unphased members, and sorts each group by agent index. Tests cover phase definitions, titles, states, counts, unphased members, and frozen inputs.

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

Change: Refactor

Possibly related PRs

Suggested labels: size:S

Suggested reviewers: bil0000, macroscopeapp

Merge Risk: Minimal · up to 89a76

No merge-blocking risk is identified for the workflow-grouping optimization.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the primary performance change: bucketing workflow phase members once.
Description check Passed The description covers the problem, solution, scope rationale, and verification results. It uses “Solution” and “Validation” instead of the template headings “Change” and “Verification,” but the requi…
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.
Full details: Description check

Explanation

The description covers the problem, solution, scope rationale, and verification results. It uses “Solution” and “Validation” instead of the template headings “Change” and “Verification,” but the required information is present.

Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between a21d788 and 89a766e.

Files selected for processing: 2
  • packages/client-runtime/src/state/subagentRuntime.test.ts
  • packages/client-runtime/src/state/subagentRuntime.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 7 remain after this review.

@hieu-lee

hieu-lee commented Nov 4, 2026

Copy link
Copy Markdown
Contributor Author

The Mobile Fingerprint Check workflow was cancelled; it did not report a test failure. The other eligible checks passed in the current-head read-back. I tried the cancelled job and then the scoped workflow rerun, but GitHub reports that repository-admin rights are required. Could a maintainer rerun https://github.com/pingdotgg/t3code/actions/runs/41931900066 ? I am keeping this check as an unmet gate rather than treating cancellation as success.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant