refactor(token-tracker-http): decompose 238-line trackTokenUsage into testable top-level functions#4937
Conversation
Extract the two inner functions from the 238-line trackTokenUsage closure
into top-level functions that accept explicit state:
- initHttpState({ streaming, compressed, contentType, contentEncoding })
Initialises the mutable tracking state object so it can be constructed
independently in tests.
- createChunkHandler(state, { requestId, provider }) → handleDecodedChunk
Returns the decoded-chunk handler; mutates `state` via explicit reference
instead of closing over outer-scope variables.
- wireListeners(proxyRes, decompressor, state, onChunk, onFinalize)
Encapsulates event-listener wiring (decompressed and uncompressed paths).
- finalizeHttpTracking(state, proxyRes, opts)
The former inner `finalizeTracking` closure, now taking all inputs as
explicit parameters so it can be unit-tested with a synthetic state object
and a minimal `{ statusCode }` proxyRes mock.
trackTokenUsage is now ~30 lines (init → create handler → wire → finalize).
The public module API is unchanged; token-tracker.js requires no updates.
New exports: createChunkHandler, finalizeHttpTracking (alongside trackTokenUsage).
New test file: token-tracker-http.unit.test.js — 18 unit tests covering both
sub-functions directly without constructing a full HTTP response stream.
All 1062 existing tests continue to pass.
Closes #4934
✅ Coverage Check PassedOverall Coverage
📁 Per-file Coverage Changes (1 files)
Coverage comparison generated by |
There was a problem hiding this comment.
Pull request overview
This PR refactors the API-proxy HTTP token usage tracker by extracting previously inner/closure-based logic from trackTokenUsage into top-level, state-driven helper functions, enabling direct unit testing without needing to construct an http.IncomingMessage stream.
Changes:
- Extracted closure-captured logic into explicit, top-level functions (
createChunkHandler,wireListeners,finalizeHttpTracking) operating on a mutablestateobject. - Simplified
trackTokenUsageinto a short orchestration function (init → handler → listener wiring → finalize). - Added a new unit test suite covering chunk handling and finalization behavior with synthetic state objects.
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/token-tracker-http.js | Decomposes trackTokenUsage internals into top-level, state-based helpers and exports key helpers for unit testing. |
| containers/api-proxy/token-tracker-http.unit.test.js | Adds unit tests for createChunkHandler and finalizeHttpTracking using synthetic state objects. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 0
🔥 Smoke Test Results — Auth mode: PAT (COPILOT_GITHUB_TOKEN)
Overall: PASS CC
|
|
Smoke Test: Copilot BYOK (Direct) Mode ✅
Status: PASS | Mode: direct BYOK (COPILOT_PROVIDER_API_KEY)
|
🔬 Smoke Test Results — PASS
PR: refactor(token-tracker-http): decompose 238-line trackTokenUsage into testable top-level functions ✅ Overall: PASS
|
|
Smoke test results:
Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "registry.npmjs.org"See Network Configuration for more information.
|
|
Smoke test results for PR #4937:
Running in direct BYOK mode (AWF_AUTH_TYPE=github-oidc + AWF_AUTH_AZURE_* + COPILOT_PROVIDER_BASE_URL) via api-proxy → Azure OpenAI (Foundry, o4-mini-aw) authenticated via Microsoft Entra Overall: PASS
|
🔬 Smoke Test: API Proxy OpenTelemetry Tracing
All 6 scenarios pass. The OTEL integration is functioning correctly on this PR.
|
🧪 Chroot Version Comparison Results
Overall: ❌ Not all tests passed Go versions match, but Python and Node.js versions differ between host and chroot environments.
|
Smoke Test Results — Services Connectivity
Overall: FAIL
|
Smoke Test Results
Overall status: FAIL Warning Firewall blocked 1 domainThe following domain was blocked by the firewall during workflow execution:
network:
allowed:
- defaults
- "localhost"See Network Configuration for more information.
|
🏗️ Build Test Suite Results
Overall: 8/8 ecosystems passed — ✅ PASS
|
trackTokenUsageintoken-tracker-http.jscontained two untestable inner functions (handleDecodedChunk,finalizeTracking) that closed over ~12 outer-scope variables, making unit testing impossible without constructing a fullhttp.IncomingMessagestream.Changes
token-tracker-http.jsExtracted all inner functions to top-level, each accepting explicit state instead of relying on closure:
initHttpState(flags)— constructs the mutable tracking state object (chunks,streamingUsage,partialLine, overflow flags, etc.)createChunkHandler(state, { requestId, provider })— returns thehandleDecodedChunkfunction; mutatesstateby reference, no outer-scope capturewireListeners(proxyRes, decompressor, state, onChunk, onFinalize)— encapsulates the compressed/uncompressed event-listener branchingfinalizeHttpTracking(state, proxyRes, opts)— the formerfinalizeTrackingclosure; takes all inputs explicitly so it's callable with a synthetic state object and a{ statusCode }mocktrackTokenUsageshrinks to ~30 lines (init → handler → wire → finalize). Public API is unchanged;token-tracker.jsrequires no updates.createChunkHandlerandfinalizeHttpTrackingare added tomodule.exportsfor direct testing.token-tracker-http.unit.test.js(new)18 unit tests for
createChunkHandlerandfinalizeHttpTrackingusing synthetic state objects — covering SSE accumulation, partial-line buffering across chunks, overflow handling, non-2xx skip, streaming flush,onUsage/onSpanEndcallbacks, and error resilience.