Skip to content
This repository was archived by the owner on Aug 6, 2026. It is now read-only.

Fix "undefined · ~NaNK tokens" in codex compaction boundary - #3379

Closed
k11kirky wants to merge 2 commits into
mainfrom
posthog-code/fix-codex-compaction-boundary-nan
Closed

Fix "undefined · ~NaNK tokens" in codex compaction boundary#3379
k11kirky wants to merge 2 commits into
mainfrom
posthog-code/fix-codex-compaction-boundary-nan

Conversation

@k11kirky

Copy link
Copy Markdown
Contributor

Problem

A user on gpt-5.6-sol saw the transcript render "Conversation compacted · undefined · ~NaNK tokens" after an auto-compaction.

The codex-app-server adapter emitted the _posthog/compact_boundary notification with only sessionId, whereas the Claude adapter includes trigger, preTokens, and contextSize. With those fields missing, CompactBoundaryView rendered ${trigger} as "undefined" and Math.round(undefined / 1000) as NaN.

Changes

  • UsageTracker retains the model context window and exposes contextSize().
  • The codex compaction boundary now sends trigger: "auto" (codex only auto-compacts), preTokens from live context occupancy, and contextSize.
  • CompactBoundaryView (and its type chain) treat trigger/preTokens as optional and omit each segment when absent — so a missing value can never render as undefined/NaN again.

Why

Reported by a user; the compaction marker was showing garbage text on codex models.

How did you test this?

  • Extended the existing codex agent test to emit a token-usage update before compaction and assert the boundary carries trigger: "auto", preTokens: 185000, contextSize: 200000. All 75 codex agent tests pass.
  • pnpm --filter @posthog/ui typecheck and @posthog/agent build pass; Biome lint clean on the changed files.

Automatic notifications

  • Publish to changelog?
  • Alert Sales and Marketing teams?

Created with PostHog Code

@trunk-io

trunk-io Bot commented Jul 13, 2026

Copy link
Copy Markdown

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

React Doctor found no issues in the changed files. 🎉

Reviewed by React Doctor for commit 0222420.

@k11kirky
k11kirky marked this pull request as ready for review July 13, 2026 11:21
@greptile-apps

greptile-apps Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Reviews (1): Last reviewed commit: "simplify compact boundary rendering and ..." | Re-trigger Greptile

@github-actions

Copy link
Copy Markdown

This PR has had no activity for 7 days and has been marked stale. We are moving to the monorepo and tightening PR staleness in preparation, so it will be closed in 7 days if no further activity occurs.

@github-actions github-actions Bot added the stale No recent changes to PR label Jul 23, 2026
posthog Bot added 2 commits July 27, 2026 20:12
The codex-app-server adapter (gpt-5.6-sol and other codex models) emitted the
`_posthog/compact_boundary` notification with only `sessionId`, unlike the Claude
adapter which includes `trigger`, `preTokens`, and `contextSize`. CompactBoundaryView
then rendered `${trigger}` as "undefined" and `Math.round(undefined / 1000)` as NaN,
producing "Conversation compacted · undefined · ~NaNK tokens".

- UsageTracker now retains the model context window (a constant, so it survives
  resetForTurn) and exposes contextSize().
- The codex boundary now sends trigger: "auto" (codex only auto-compacts), preTokens
  from the live context occupancy, and contextSize.
- CompactBoundaryView and its type chain treat trigger/preTokens as optional and omit
  each segment when absent, so a missing value can never render as undefined/NaN again.

Generated-By: PostHog Code
Task-Id: d044b358-be1b-4665-8d32-c28a9ce1badb
Omit preTokens instead of sending 0 when no usage reading exists, reuse formatTokensCompact, and derive one shared detail string for both render branches.

Generated-By: PostHog Code
Task-Id: 71484330-f52a-49bb-a528-5c708028bbf9
@posthog
posthog Bot force-pushed the posthog-code/fix-codex-compaction-boundary-nan branch from 8e9f3bf to 0222420 Compare July 27, 2026 20:12
@posthog

posthog Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

🤖 CI You Later — rebased this branch onto main to clear the merge conflict (DIRTY → clean).

Conflicts were in 4 files. How each was resolved:

  • packages/agent/.../usage-tracker.ts — both sides were additive and non-overlapping: main refactored lastTurn (added thoughtTokens/totalTokens), this PR added contextWindow tracking + contextSize(). Kept both.
  • packages/ui/.../CompactBoundaryView.tsx, buildConversationItems.ts, SessionUpdateView.tsx — took main verbatim. main already landed the renderer-side fix this PR was making: CompactBoundaryMetadata (in types.ts) has optional trigger/preTokens, and getCompactBoundaryMetadata / formatCompactBoundaryLabel omit each segment when absent, so undefined / ~NaNK can no longer render. main also has a test file (CompactBoundaryView.test.ts) covering those exported helpers, and renamed markCompactingStatusCompletemarkRuntimeStatusComplete(b, "compacting"), so keeping the PR's side would have broken compilation and dropped tests.

Net effect: the remaining diff vs main is agent-side only (26 insertions across 3 files) — the codex adapter now sends trigger: "auto", preTokens, and contextSize on the compaction boundary, plus the tracker/test support for it. The UI files are now byte-identical to main.

Verified locally before pushing: pnpm typecheck (24/24 packages), codex-app-server-agent.test.ts + usage-tracker.test.ts (113 passed), CompactBoundaryView.test.ts + buildConversationItems.test.ts (39 passed), Biome clean on changed files.

⚠️ For @k11kirky: the PR description still describes the renderer changes, which are now redundant with main — worth trimming before merge. Also note this rebase rewrote the two commits (8e9f3bf0222420).

@charlesvien

Copy link
Copy Markdown
Member

Closing stale PRs ahead of Friday's monorepo migration. If this is a mistake, rebase and reopen for review.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

stale No recent changes to PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants