Skip to content

fix(agents): preserve embedded OpenAI completions usage#96523

Merged
vincentkoc merged 1 commit into
openclaw:mainfrom
ly85206559:codex/fix-embedded-openai-completions-usage
Jul 2, 2026
Merged

fix(agents): preserve embedded OpenAI completions usage#96523
vincentkoc merged 1 commit into
openclaw:mainfrom
ly85206559:codex/fix-embedded-openai-completions-usage

Conversation

@ly85206559

@ly85206559 ly85206559 commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

What Problem This Solves

Refs #96463. This PR fixes the embedded runner usage-accounting path; any broader custom-provider request-policy decision can remain tracked on the issue.

When openclaw agent --local --json uses an OpenAI-compatible provider through the embedded runner, the provider stream can report nonzero token usage while the final assistant snapshot still carries a zeroed usage object. The run metadata then treated that zero object as authoritative, so agentMeta.usage, agentMeta.lastCallUsage, session listings, and related JSON output stayed at 0 even though the stream had usage.

Why This Change Was Made

The OpenAI completions provider already parses streaming usage chunks. The embedded run aggregation path now treats a zeroed assistant usage snapshot as missing for run-level accounting, keeps latest-call usage separate from accumulated attempt totals, and preserves the streamed nonzero usage on the final assistant snapshot when the provider's final message snapshot is zeroed.

The final metadata helper follows the same rule for lastCallUsage, so zero snapshots no longer mask known nonzero usage in either returned JSON metadata or the persisted session assistant artifact.

User Impact

Custom OpenAI-compatible providers using the embedded runner now report nonzero token usage in --json output and persisted assistant session records when the provider stream includes usage, instead of always showing zeros.

Evidence

  • Added focused regression coverage in src/agents/embedded-agent-subscribe.handlers.messages.test.ts asserting that a zeroed final assistant message object is updated in place to usage.totalTokens: 12 before noteLastAssistant / recordAssistantUsage consume it.
  • node scripts/run-vitest.mjs src/agents/embedded-agent-subscribe.handlers.messages.test.ts src/agents/embedded-agent-runner/run/helpers.test.ts src/agents/embedded-agent-runner/usage-reporting.test.ts src/llm/providers/openai-completions.test.ts
  • node_modules\.bin\oxlint.CMD src\agents\embedded-agent-subscribe.handlers.messages.ts src\agents\embedded-agent-subscribe.handlers.messages.test.ts
  • node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfo
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo
  • git diff --check

Loopback OpenAI-compatible provider proof, redacted:

OPENCLAW_CONFIG_PATH=<temp>\home\.openclaw\openclaw.json `
OPENCLAW_STATE_DIR=<temp>\state `
OPENCLAW_AGENT_DIR=<temp>\state\agents\main `
node scripts/run-node.mjs agent --agent main `
  --session-key loopback-usage-proof `
  --model loopback/loopback-model `
  --message "Say loopback usage ok." `
  --local --json --timeout 60

Observed --json output excerpt:

{
  "payloads": [{ "text": "loopback usage ok", "mediaUrl": null }],
  "meta": {
    "agentMeta": {
      "provider": "loopback",
      "model": "loopback-model",
      "usage": { "input": 7, "output": 5, "total": 12 },
      "lastCallUsage": {
        "input": 7,
        "output": 5,
        "cacheRead": 0,
        "cacheWrite": 0,
        "total": 12
      },
      "promptTokens": 7
    },
    "finalAssistantVisibleText": "loopback usage ok"
  }
}

Observed session JSONL assistant artifact excerpt:

{
  "type": "message",
  "message": {
    "role": "assistant",
    "api": "openai-completions",
    "provider": "loopback",
    "model": "loopback-model",
    "usage": {
      "input": 7,
      "output": 5,
      "cacheRead": 0,
      "cacheWrite": 0,
      "totalTokens": 12,
      "cost": { "input": 0, "output": 0, "cacheRead": 0, "cacheWrite": 0, "total": 0 }
    }
  }
}

Loopback server log confirmed the embedded runner called POST /v1/chat/completions on the custom OpenAI-compatible provider and received an SSE usage chunk with prompt_tokens: 7, completion_tokens: 5, and total_tokens: 12.

Autoreview was attempted with python .agents\skills\autoreview\scripts\autoreview --mode branch --base origin/main. One run hit the Codex account usage limit, and later retries timed out after 10 minutes without returning findings.

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: S triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. labels Jun 24, 2026
@clawsweeper

clawsweeper Bot commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs maintainer review before merge. Reviewed July 2, 2026, 2:54 AM ET / 06:54 UTC.

Summary
The PR preserves nonzero streamed OpenAI-completions usage in embedded agent metadata and persisted assistant session snapshots when a final assistant usage snapshot is zeroed.

PR surface: Source +138, Tests +244. Total +382 across 9 files.

Reproducibility: yes. at source level: current main lets a present zeroed final assistant snapshot mask nonzero streamed or current-attempt usage in embedded run metadata and message-end persistence. I did not run a live current-main custom provider in this read-only review.

Review metrics: 1 noteworthy metric.

  • Usage Accounting Surfaces: 3 fallback sources changed, 1 persistence repair added. The diff changes latest-call selection, accumulated run usage fallback, final metadata construction, and assistant snapshot persistence, which are the maintainer-sensitive session-state surfaces.

Stored data model
Persistent data-model change detected: serialized state: src/agents/embedded-agent-runner/usage-reporting.test.ts. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #96463
Summary: This PR is a narrow candidate fix for the embedded usage-accounting portion of the canonical custom OpenAI-compatible zero-usage issue, while broader provider request-policy remains on the issue.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • none.

Risk before merge

  • [P1] The PR changes how embedded runs choose usage, last-call usage, prompt-token metadata, and persisted assistant snapshots; maintainers should explicitly accept that known streamed usage beats a zeroed final snapshot.
  • [P1] The linked canonical issue still owns the broader custom-provider request-policy question, so merging this PR should not be treated as resolving every generic OpenAI-compatible usage case.

Maintainer options:

  1. Accept Zero-Snapshot Override (recommended)
    Maintainers can accept that known nonzero streamed usage should override a missing or zeroed final assistant snapshot for embedded metadata and session persistence.
  2. Keep Provider Policy Separate
    Land this accounting repair without broadening the branch into default stream_options.include_usage behavior for every custom provider.
  3. Pause If Scope Must Expand
    If maintainers want one PR to solve the full custom-provider request-policy issue, pause this branch and require a broader compatibility design first.

Next step before merge

  • [P2] Maintainers need to accept the narrowed session-state accounting behavior and keep the broader provider request-policy work on the linked issue; there is no narrow automated repair left.

Security
Cleared: The diff changes in-process TypeScript usage accounting and tests only; no dependency, workflow, secret, permission, or supply-chain concern was found.

Review details

Best possible solution:

Land the narrowed usage-preservation fix if maintainers accept the session-state behavior, and keep #96463 open for the broader provider request-policy decision.

Do we have a high-confidence way to reproduce the issue?

Yes at source level: current main lets a present zeroed final assistant snapshot mask nonzero streamed or current-attempt usage in embedded run metadata and message-end persistence. I did not run a live current-main custom provider in this read-only review.

Is this the best way to solve the issue?

Yes for the narrowed fix: the PR repairs the local owner paths that consume streamed usage, build final metadata, and persist the assistant snapshot. The broader default request-policy question belongs on #96463, not in this branch.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against be5906e19dd7.

Label changes

Label justifications:

  • P2: The PR targets a bounded embedded-agent usage accounting bug with session metadata impact and no crash or core runtime outage.
  • merge-risk: 🚨 session-state: The diff changes how embedded runs choose usage, last-call usage, prompt-token metadata, and persisted assistant snapshots.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes redacted live CLI output, persisted assistant session usage output, and loopback server-log evidence for the OpenAI-compatible provider path after the fix; the current Real behavior proof check is green.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes redacted live CLI output, persisted assistant session usage output, and loopback server-log evidence for the OpenAI-compatible provider path after the fix; the current Real behavior proof check is green.
Evidence reviewed

PR surface:

Source +138, Tests +244. Total +382 across 9 files.

View PR surface stats
Area Files Added Removed Net
Source 5 146 8 +138
Tests 4 244 0 +244
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 9 390 8 +382

What I checked:

Likely related people:

  • vincentkoc: Recent history touches embedded run session handling, provider streaming, and message-handler helper surfaces adjacent to this PR, and the current PR head commit is authored by this account. (role: recent area contributor; confidence: high; commits: 31a65e0647ae, 70de1047b8ed, 77d0deedf2dc; files: src/agents/embedded-agent-runner/run.ts, src/agents/embedded-agent-subscribe.handlers.messages.ts, src/llm/providers/openai-completions.ts)
  • steipete: History shows prior work on shared usage normalization, streamed timings recovery, and OpenAI reasoning token usage in the same accounting area. (role: usage helper and streamed-usage contributor; confidence: medium; commits: 2908190ba208, 0ddf51cf7106, e533ff4c4a11; files: src/agents/usage.ts, src/agents/embedded-agent-runner/run/helpers.ts, src/llm/providers/openai-completions.ts)
  • chenhaoqiang: Recent history includes performance work in the embedded message-handler and run paths that share the serialized event and usage state boundary touched here. (role: recent message-handler contributor; confidence: medium; commits: 1876e3e1c13b; files: src/agents/embedded-agent-subscribe.handlers.messages.ts, src/agents/embedded-agent-runner/run.ts)
  • xuruiray: The broader linked issue depends on the OpenAI-completions streaming-usage compatibility gate introduced in this area, which this PR intentionally leaves out of scope. (role: adjacent streaming-usage compatibility contributor; confidence: medium; commits: ff6b38750e7f; files: src/agents/openai-completions-compat.ts, src/agents/openai-transport-stream.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 24, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. labels Jun 24, 2026
@ly85206559

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed the latest review on head 8d67883f89f: the streamed nonzero usage is now preserved on the final assistant/session snapshot when the final provider message carries a zeroed usage object. I also updated the PR body with redacted loopback openclaw agent --local --json proof showing nonzero agentMeta.usage, lastCallUsage, and the persisted session JSONL assistant usage artifact.

@clawsweeper

clawsweeper Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added proof: sufficient ClawSweeper judged the real behavior proof convincing. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. and removed status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jun 27, 2026
@ly85206559

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed the session-owned message finding on head 0a478b1af9d49432987038dfcd7ab77d88d28ae1: recovered streamed usage is now written back onto the original assistant message object as a standard assistant usage snapshot (totalTokens + zeroed cost) before the message is recorded. I also added focused regression coverage in src/agents/embedded-agent-subscribe.handlers.messages.test.ts that starts with a zeroed final assistant message and asserts the same object is updated in place before noteLastAssistant / recordAssistantUsage consume it.

Fresh local proof after the fix:

  • node scripts/run-vitest.mjs src/agents/embedded-agent-subscribe.handlers.messages.test.ts src/agents/embedded-agent-runner/run/helpers.test.ts src/agents/embedded-agent-runner/usage-reporting.test.ts src/llm/providers/openai-completions.test.ts
  • node_modules\.bin\oxlint.CMD src\agents\embedded-agent-subscribe.handlers.messages.ts src\agents\embedded-agent-subscribe.handlers.messages.test.ts
  • git diff --check

@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 27, 2026
@clawsweeper

clawsweeper Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@ly85206559

ly85206559 commented Jun 27, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Addressed the latest type/lint findings on head 4b520d625b4711c5069e70f75fa81df864717d0c:

  • preservePendingAssistantUsage now accepts and returns the already-narrowed AssistantMessage, so downstream assistant-only helpers keep their type narrowing.
  • The new regression test now casts noteLastAssistant through unknown before reading mock calls, matching the safe test mock pattern.
  • The PR body now uses Refs #96463 and explicitly scopes this PR to the embedded runner usage-accounting fix, so the broader custom-provider request-policy question can remain on the issue if maintainers want it tracked separately.
  • The PR body was rewritten without a UTF-8 BOM and includes fresh core prod/test typecheck proof.

Fresh local proof:

  • node scripts/run-vitest.mjs src/agents/embedded-agent-subscribe.handlers.messages.test.ts src/agents/embedded-agent-runner/run/helpers.test.ts src/agents/embedded-agent-runner/usage-reporting.test.ts src/llm/providers/openai-completions.test.ts
  • node_modules\\.bin\\oxlint.CMD src\\agents\\embedded-agent-subscribe.handlers.messages.ts src\\agents\\embedded-agent-subscribe.handlers.messages.test.ts
  • node scripts/run-tsgo.mjs -p tsconfig.core.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core.tsbuildinfo
  • node scripts/run-tsgo.mjs -p test/tsconfig/tsconfig.core.test.json --incremental --tsBuildInfoFile .artifacts/tsgo-cache/core-test.tsbuildinfo
  • git diff --check

@clawsweeper

clawsweeper Bot commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. labels Jun 27, 2026
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-pr-context Candidate: external PR body lacks required problem context or evidence. label Jun 27, 2026
@ly85206559

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

Follow-up after the previous review on head 4b520d625b4711c5069e70f75fa81df864717d0c: the PR body has been rewritten with normal Markdown section line breaks, and the latest Real behavior proof check is now passing at https://github.com/openclaw/openclaw/actions/runs/28290963260/job/83822766352. The previously reported body-parser blocker should be stale now; code findings were addressed in 4b520d625b4711c5069e70f75fa81df864717d0c and current CI type/lint gates are green.

Co-authored-by: ben.li <li.yang6@xydigit.com>
@vincentkoc
vincentkoc force-pushed the codex/fix-embedded-openai-completions-usage branch from 279638e to a66a69a Compare July 2, 2026 06:57
@vincentkoc

Copy link
Copy Markdown
Member

Maintainer repair is land-ready on exact head a66a69a3666bfe1ce3904380a5009dc1861c800f.

What changed:

  • preserves nonzero streamed usage before AgentSession synchronously persists the final assistant message;
  • keeps current-attempt usage distinct from carried/historical metadata across retries;
  • preserves reasoning tokens and excludes transcript-only delivery mirrors/gateway-injected messages.

Proof:

  • Testbox tbx_01kwgn5e3sp7h9v2703b2acsvy: 69/69 focused embedded-runner/message-lifecycle tests passed;
  • scoped pnpm check:changed passed core/core-test tsgo, selected oxlint shards, and repository guards for all 9 changed files;
  • final autoreview: no accepted/actionable findings, confidence 0.90;
  • exact-head hosted CI: https://github.com/openclaw/openclaw/actions/runs/28571560597 (success);
  • final OPENCLAW_TESTBOX=1 scripts/pr prepare-run 96523 passed.

No provider live test was required because this repair does not change provider request shaping, defaults, config, or endpoint behavior. It only preserves usage already delivered by the stream. Issue #96463 remains open for the separate product decision about whether generic custom OpenAI-compatible providers should request stream_options.include_usage by default.

@vincentkoc
vincentkoc merged commit cb44f40 into openclaw:main Jul 2, 2026
95 checks passed
@vincentkoc

Copy link
Copy Markdown
Member

Merged via squash.

github-actions Bot pushed a commit to Desicool/openclaw that referenced this pull request Jul 3, 2026
Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>
chenyangjun-xy pushed a commit to chenyangjun-xy/openclaw that referenced this pull request Jul 3, 2026
Co-authored-by: Vincent Koc <25068+vincentkoc@users.noreply.github.com>
@ly85206559
ly85206559 deleted the codex/fix-embedded-openai-completions-usage branch July 6, 2026 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agents Agent runtime and tooling merge-risk: 🚨 session-state 🚨 May lose, corrupt, stale, or mis-associate session, agent, or context state. P2 Normal backlog priority with limited blast radius. proof: sufficient ClawSweeper judged the real behavior proof convincing. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. size: M status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants