🔖 fix: Split Activity Phases at Substantial Text (#14785)

* fix(api): split phases at substantial text

* tune(api): split phases after 200 text chars

* fix(api): reanchor substantial text boundaries

* test(api): type multi-phase payload captures

* fix(api): preserve activity phase boundaries

* fix(api): anchor retained phase partitions

* fix(api): persist phase partition anchors

* fix(api): harden activity phase boundaries

* fix(api): preserve bounded phase partitions

* fix: preserve final and delayed phase content

* refactor(api): partition phase state at one boundary

Boundary closure split fifteen separately-maintained fields by hand, and
each fix partitioned one more while the next stayed unguarded. Fold the
overflow bookkeeping into the tracked activity list so every counted
activity carries a position, and route the split through a single
partitionAt that returns both sides.

Counts are now summed from the partition instead of reconstructed by
subtraction, so a run past the anchor budget reports every activity it
performed rather than the truncated window. Snapshots move to version 3;
the reader still accepts versions 1 and 2 and rebuilds their unpositioned
remainder as a bounded anchor, dropping it when its evidence is stale.

Adds a boundary-conservation property covering every split point.

* fix(client): drop empty phase content segments

Late-child recovery can strip every index from a segment it already
claimed, leaving a content segment with no parts. Each one still mounts a
nested ContentParts that renders nothing, and it broke the exact-segment
expectation in the late-child regression from 831a00353.

Route the four content pushes through one guard that skips index-less
segments, matching the existing splice of fully recovered segments.

* fix(api): keep the run's answer outside the collapsed phase

The substantial-text boundary replaced completion's final-text boundary
outright, so a short reply from a provider that emits no phase metadata
was folded into the collapsed parent. That is the deterministic e2e
failure at activity-phases.spec.ts:182 and codex's short-final-answer
findings; 831a00353 fixed only the path where the provider labels the
step final_answer.

Restore the completion boundary at the last materialized visible text
whatever its length. Length now decides only whether intermediate text
earns a boundary, and semantic commentary still stays inside. The
"later work" check shares one predicate with partitionAt so the two
cannot drift.

Also splits a legacy v1/v2 remainder across the positions its saved tool
anchors still materialize at, each carrying its own id so it can be
located, and merges over-cap anchors by closest pair into the earlier
position instead of folding the oldest forward.

* fix(api): clear resolved anchors and keep folded agents

Two findings from the latest review:

A resumed activity whose tool was missing at construction kept its high
fallback anchor after that tool materialized at a lower index, so the
partition rejected it at any boundary below the stale value and pushed
pre-boundary work into the following phase. Drop the anchor once every
tracked call has materialized.

Folding anchors past the cap spread only the surviving side, silently
dropping the other's agent. close() now derives both marker attribution
and the summarizer payload from the partitioned activities, so a merged
anchor carries the union instead.

Both regressions are mutation-checked against their own fix.

* fix(api): anchor live batches awaiting materialization

A batch tracked after its child-label slot is reserved but before its
tool call reaches the shared array had no materialized position, and the
partition read "nothing materialized" as "happened earlier". A boundary
between the two then counted the batch in the earlier phase while ending
before its eventual tool call, stranding the tool outside its parent.

Record the tracked start as an unresolved anchor in that window so the
existing retain branch keeps the batch on its own side. Using the plain
fallback index instead is wrong: restored evidence-less activities carry
a synthesized index, not a position.

Regression is mutation-checked against its own fix.

* perf(api): partition in one pass and reanchor filtered batches

Dropping a batch's already-covered calls leaves a different activity
behind, but the batch position was still the covered call's index. The
survivor therefore inherited a position inside an emitted phase and was
consumed by it instead of being held for its own. Re-derive the start
from the retained ids, which also restores the unresolved-anchor signal
when none of them have materialized.

The boundary partition also classified every activity twice and rescanned
retained ones to reanchor, walking the shared content array several times
per activity per boundary. Build both sides in one pass with the
materialized tool indices computed once and threaded into the predicate.

Regression is mutation-checked; an earlier version of it was vacuous
because the tool materialized before completion, converging both paths.

* perf(api): carry tool indices through boundary resolution

The previous pass cached the materialized indices only in the partition
loop, so resolution still scanned for the batch start and again for the
fully-materialized check, and an empty result triggered a third scan
inside the boundary predicate.

Walk the shared content array once per activity and carry the indices
through resolution, classification, and reanchoring. findTrackedToolStart
becomes its own first element and is dropped.

* fix: trust rendered position over registration order

Three findings from the latest review:

Context partitioning only consulted the rendered index when the activity
position tied the closing count, so a parallel lane registered before the
closing tool hooks was assigned to the earlier phase despite rendering
after the boundary. An activity position is registration order; a
materialized index is proof, and now wins whenever it has one.

Snapshot restore bound pending reasoning to the first part sharing its
80-character anchor, which could replay a still-pending lane on the
earlier side of a boundary and delete it. An ambiguous anchor is treated
as unresolved.

Recovering the only filled child label out of a phase segment left its
hasContent flag set, rendering an expandable card with an empty body.

The context regression is mutation-checked against its own fix.

* fix(api): decide context by proven position, both directions

The previous change let a rendered index override registration order only
when it proved the text was after the boundary, and trusted that index
even when it was not provably this entry's.

Both gaps were reachable. Context registered after work that already
rendered ahead of it was retained despite rendering before the boundary,
and a restored entry with no step id whose excerpt repeats after the
boundary matched the later occurrence and moved to the wrong phase.

Locating now reports whether the position is authoritative — anchored by
a step index or a unique text match — and only then decides, in both
directions. Otherwise the saved activity position stands.

Each regression is mutation-checked against its own direction.

* fix(api): carry unresolved positions through folded anchors

Folding two anchors spread only the surviving side, dropping the later
one's unresolved fallback. A boundary between them then saw just the
earlier materialized tool index and closed the whole merged count,
counting work whose tool call had not appeared and leaving that call
outside its parent.

Carry the later fallback into the merged anchor; resolution already
clears it once every retained id materializes.

Regression is mutation-checked against its own fix.

* fix(client): keep phase headers recovery did not empty

A completed phase can carry no children after compaction — its summary
header is the whole segment. Recovery spliced any segment left with no
retained indices, so a later marker deleted that header even though it
recovered nothing from it.

Only drop a segment recovery actually emptied, not one that arrived
empty. Regression is mutation-checked against its own fix.

client/src/utils/__tests__/activityLabels.spec.ts

@@ -234,6 +234,142 @@ describe('groupActivityPhases', () => {
     expect(lastVisibleContentIdx([tool, tool, final, phase as never])).toBe(2);
   });
 
+  it('keeps an emptied earlier phase header when a later marker recovers nothing', () => {
+    const laterTool = {
+      type: ContentTypes.TOOL_CALL,
+      tool_call: { id: 'later', name: 'web_search', args: '{}', output: 'ok' },
+    } as unknown as TMessageContentParts;
+    /** Compaction left this completed phase with no children, so its summary
+     *  header is the entire segment. A later marker recovers nothing from it. */
+    const emptied = labelPart({ activity_label: 'Completed the compacted phase', pending: false });
+    Object.assign(emptied, {
+      activity_label_type: 'phase',
+      activity_start_index: 0,
+      activity_end_index: 0,
+      activity_count: 2,
+    });
+    const later = labelPart({ activity_label: 'Completed the later phase', pending: false });
+    Object.assign(later, {
+      activity_label_type: 'phase',
+      activity_start_index: 0,
+      activity_end_index: 2,
+      activity_count: 2,
+    });
+    const content = [emptied as never, laterTool, later as never];
+
+    const segments = groupActivityPhases(content);
+
+    expect(segments?.filter((segment) => segment.type === 'phase')).toHaveLength(2);
+  });
+
+  it('groups multiple logical spans when an earlier phase marker arrives late', () => {
+    const first = labelPart({ activity_label: 'Completed the first phase', pending: false });
+    Object.assign(first, {
+      activity_label_type: 'phase',
+      activity_start_index: 0,
+      activity_end_index: 2,
+      activity_count: 2,
+    });
+    const second = labelPart({ activity_label: 'Completed the second phase', pending: false });
+    Object.assign(second, {
+      activity_label_type: 'phase',
+      activity_start_index: 3,
+      activity_end_index: 6,
+      activity_count: 2,
+    });
+    const boundary = {
+      type: ContentTypes.TEXT,
+      text: 'Substantial result',
+    } as TMessageContentParts;
+    const final = { type: ContentTypes.TEXT, text: 'Final result' } as TMessageContentParts;
+    const content = [tool, tool, boundary, tool, first as never, tool, final, second as never];
+
+    const segments = groupActivityPhases(content);
+
+    expect(segments).toEqual([
+      expect.objectContaining({
+        type: 'phase',
+        startIndex: 0,
+        labelIndex: 4,
+        contentIndices: [0, 1],
+      }),
+      expect.objectContaining({
+        type: 'content',
+        startIndex: 2,
+        contentIndices: [2],
+      }),
+      expect.objectContaining({
+        type: 'phase',
+        startIndex: 3,
+        labelIndex: 7,
+        contentIndices: [3, 5],
+      }),
+      expect.objectContaining({
+        type: 'content',
+        startIndex: 6,
+        contentIndices: [6],
+      }),
+    ]);
+  });
+
+  it('moves a late child label from an earlier phase into its declared later span', () => {
+    const firstTool = {
+      type: ContentTypes.TOOL_CALL,
+      tool_call: { id: 'first', name: 'web_search', args: '{}', output: 'ok' },
+    } as unknown as TMessageContentParts;
+    const laterTool = {
+      type: ContentTypes.TOOL_CALL,
+      tool_call: { id: 'later', name: 'web_search', args: '{}', output: 'ok' },
+    } as unknown as TMessageContentParts;
+    const lateChild = labelPart({
+      activity_label: 'Recorded the later result',
+      pending: false,
+      tool_call_ids: ['later'],
+    });
+    const first = labelPart({ activity_label: 'Completed the first phase', pending: false });
+    Object.assign(first, {
+      activity_label_type: 'phase',
+      activity_start_index: 0,
+      activity_end_index: 1,
+      activity_count: 2,
+    });
+    const second = labelPart({ activity_label: 'Completed the second phase', pending: false });
+    Object.assign(second, {
+      activity_label_type: 'phase',
+      activity_start_index: 2,
+      activity_end_index: 4,
+      activity_count: 2,
+    });
+    const boundary = { type: ContentTypes.TEXT, text: 'Boundary' } as TMessageContentParts;
+    const content = [
+      firstTool,
+      boundary,
+      laterTool,
+      lateChild as never,
+      first as never,
+      second as never,
+    ];
+
+    const segments = groupActivityPhases(content);
+
+    expect(segments).toEqual([
+      expect.objectContaining({
+        type: 'phase',
+        labelIndex: 4,
+        contentIndices: [0],
+      }),
+      expect.objectContaining({
+        type: 'content',
+        contentIndices: [1],
+      }),
+      expect.objectContaining({
+        type: 'phase',
+        labelIndex: 5,
+        contentIndices: [2, 3],
+      }),
+    ]);
+  });
+
   it('keeps a late child label in the phase while leaving final text outside', () => {
     const child = labelPart({
       activity_label: 'Recorded the delayed child result',

client/src/utils/activityLabels.ts

@@ -177,19 +177,88 @@ export function groupActivityPhases(
     segment.contentIndices.push(partIndex);
     segment.hasContent ||= isVisibleContentPart(child);
   };
+  /** Recovery can empty a span it already claimed. An index-less segment
+   *  renders nothing but still mounts a nested `ContentParts`, so drop it the
+   *  same way a fully recovered segment is spliced out below. */
+  const pushContent = (segment: ReturnType<typeof collect>, startIndex: number) => {
+    if (segment.contentIndices.length === 0) {
+      return;
+    }
+    segments.push({
+      type: 'content',
+      content: segment.content,
+      contentIndices: segment.contentIndices,
+      startIndex,
+    });
+  };
   /** Phase markers and defined content indexes are both sorted. Walk them in
    *  lockstep so every ordinary part is classified once, even when a custom
    *  max permits many parent phases in one long response. */
   for (const { part, index } of completed) {
     if (!part) continue;
-    const start = Math.max(
-      cursor,
-      Math.min(index, Math.max(0, part.activity_start_index ?? index)),
-    );
+    const start = Math.min(index, Math.max(0, part.activity_start_index ?? index));
     const end = Math.max(start, Math.min(index, Math.max(0, part.activity_end_index ?? index)));
     const adjacent = collect();
     const phase = collect();
     const trailing = collect();
+    /** A boundary may resolve after a higher-index parallel activity has
+     *  already rendered. Recover that activity from an earlier adjacent
+     *  segment so a later phase marker can still claim its declared span. */
+    if (start < cursor) {
+      const recoveredIndices: number[] = [];
+      const deferredTrailingIndices: number[] = [];
+      for (let segmentIndex = segments.length - 1; segmentIndex >= 0; segmentIndex -= 1) {
+        const segment = segments[segmentIndex];
+        if (segment.type !== 'content' && segment.type !== 'phase') {
+          continue;
+        }
+        const retainedContent: Array<TMessageContentParts | undefined> = [];
+        const retainedIndices: number[] = [];
+        for (
+          let childPosition = 0;
+          childPosition < segment.contentIndices.length;
+          childPosition += 1
+        ) {
+          const childIndex = segment.contentIndices[childPosition];
+          const child = segment.content[childPosition];
+          const canRecover = segment.type === 'content' || getBatchActivityLabelPart(child) != null;
+          if (canRecover && childIndex >= start && childIndex < end) {
+            recoveredIndices.push(childIndex);
+          } else if (canRecover && childIndex >= end) {
+            deferredTrailingIndices.push(childIndex);
+          } else {
+            retainedContent.push(child);
+            retainedIndices.push(childIndex);
+          }
+        }
+        if (retainedIndices.length === 0) {
+          /** A completed phase can legitimately carry no children after
+           *  compaction — its summary header is the whole segment. Only drop
+           *  what recovery actually emptied, not what arrived empty. */
+          if (segment.contentIndices.length > 0) {
+            segments.splice(segmentIndex, 1);
+          }
+        } else {
+          segment.content = retainedContent;
+          segment.contentIndices = retainedIndices;
+          segment.startIndex = retainedIndices[0];
+          if (segment.type === 'phase') {
+            /** Recovery can take the only filled label and leave blank
+             *  reservations behind. A stale flag renders an expandable card
+             *  with nothing in it instead of the compact header. */
+            segment.hasContent = retainedContent.some(isVisibleContentPart);
+          }
+        }
+      }
+      recoveredIndices.sort((a, b) => a - b);
+      for (const recoveredIndex of recoveredIndices) {
+        append(phase, recoveredIndex);
+      }
+      deferredTrailingIndices.sort((a, b) => a - b);
+      for (const trailingIndex of deferredTrailingIndices) {
+        append(trailing, trailingIndex);
+      }
+    }
     while (definedPosition < definedIndices.length && definedIndices[definedPosition] < index) {
       const childIndex = definedIndices[definedPosition];
       definedPosition += 1;
@@ -208,12 +277,7 @@ export function groupActivityPhases(
       definedPosition += 1;
     }
     if (start > cursor) {
-      segments.push({
-        type: 'content',
-        content: adjacent.content,
-        contentIndices: adjacent.contentIndices,
-        startIndex: cursor,
-      });
+      pushContent(adjacent, cursor);
     }
     const labelText = getActivityLabelText(part);
     if (labelText) {
@@ -230,20 +294,10 @@ export function groupActivityPhases(
       /** A failed/empty parent stays visually feature-off, but its bounds are
        *  still authoritative: delayed child labels must move back beside the
        *  tools they describe instead of rendering after the final answer. */
-      segments.push({
-        type: 'content',
-        content: phase.content,
-        contentIndices: phase.contentIndices,
-        startIndex: start,
-      });
+      pushContent(phase, start);
     }
     if (end < index) {
-      segments.push({
-        type: 'content',
-        content: trailing.content,
-        contentIndices: trailing.contentIndices,
-        startIndex: end,
-      });
+      pushContent(trailing, end);
     }
     cursor = index + 1;
   }
@@ -256,12 +310,7 @@ export function groupActivityPhases(
         append(adjacent, childIndex);
       }
     }
-    segments.push({
-      type: 'content',
-      content: adjacent.content,
-      contentIndices: adjacent.contentIndices,
-      startIndex: cursor,
-    });
+    pushContent(adjacent, cursor);
   }
   return segments;
 }