Skip to content

fix: coalesce cost polling without changing tracker API - #198

Draft
mldangelo-oai wants to merge 3 commits into
mainfrom
mdangelo/codex/coalesce-cost-polling
Draft

fix: coalesce cost polling without changing tracker API#198
mldangelo-oai wants to merge 3 commits into
mainfrom
mdangelo/codex/coalesce-cost-polling

Conversation

@mldangelo-oai

Copy link
Copy Markdown
Collaborator

Summary

Fixes #31. Builds directly on #177 and preserves @SiavashShams's original contributor-authored commit.

  • Coalesce overlapping cost-polling interval ticks inside ScanCostTracker.start() instead of replacing the existing refresh serializer.
  • Preserve the existing one-argument tracker constructor and remove the test-only refresh-task injection added by Coalesce overlapping cost refreshes #177.
  • Run at most one polling traversal at a time, remember whether any interval ticks arrived during that traversal, and immediately perform one follow-up while tracking remains active.
  • Keep stop() authoritative: stopping clears the timer, suppresses any stale follow-up, and serializes exactly one final session refresh behind the in-flight poll.
  • Deliver a failed poll to onError exactly once and allow the coalesced follow-up to recover normally.
  • Exercise actual 100 ms timer ticks, real session JSONL files, bounded shutdown, failed traversal recovery, and deduplicated error reporting without widening production APIs for tests.

Why

Each budgeted scan polls the Codex session tree every 100 ms. On main, every tick appends another full traversal to the refresh promise chain when filesystem work takes longer than the interval. The result is an unbounded backlog and unnecessarily long scan shutdown.

The existing refresh serializer already provides the ordering required by explicit refreshes and stop(). Coalescing only the interval producer fixes the actual source of the backlog while retaining those established semantics and avoiding a second scheduler.

With 3,000 real session files and no injected filesystem latency, a representative local run produced:

Implementation Session traversals Session files opened Shutdown wait
Current main 5 15,000 1,577 ms
This PR 3 9,000 363 ms

The new polling regression fails against unmodified main: four overlapping polling requests are observed where the fixed implementation permits one.

Validation

  • Full pinned Bun 1.3.14 suite: 717 passed, 5 expected platform/integration skips, 0 failed across 32 files.
  • Focused cost-tracker suite: 15 passed.
  • Focused cost-tracker compatibility checks on Node.js 22, 24, and 26.
  • pnpm run types.
  • pnpm run format.
  • pnpm run build.
  • pnpm pack and complete package-contract validation: 179 verified entries.
  • Built SDK import/cost-estimation smoke and CLI version smoke.
  • git diff --check.

This change addresses refresh backlog and shutdown latency. It does not claim to eliminate the separate cost of recursively discovering historical session files.

Copy link
Copy Markdown
Collaborator Author

@codex review

Please review the exact current head 9c6a5889387d5a8490dae25a481a8271eabb8727, with particular attention to overlapping polling ticks, failed refresh recovery, deduplicated error reporting, shutdown ordering, and cost-limit enforcement.

@github-actions github-actions Bot added the bug Something isn't working label Aug 1, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Nice work!

Reviewed commit: 9c6a588938

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@chatgpt-codex-connector

Copy link
Copy Markdown

Security review completed. No security issues were found in this pull request.

Reviewed commit: 9c6a588938

View security finding report

Only the user who started this review can view the report in Codex.

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

  • You comment "@codex security review"
  • A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

maxCostUsd polling queues full session-tree rescans and can delay budget enforcement

2 participants