refactor: extract shared token-tracker budget helpers#4780
Conversation
Extract warnCacheReadRollupMismatch() and mergeBudgetFields() into token-tracker-shared.js to eliminate duplicated code between token-tracker-http.js and token-tracker-ws.js. Fixes #4711 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
✅ 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 token trackers by extracting duplicated “budget field merge” and “cache-read rollup mismatch warning” logic into a new shared helper module, keeping the HTTP and WebSocket trackers behaviorally consistent while reducing duplication.
Changes:
- Added
containers/api-proxy/token-tracker-shared.jswithwarnCacheReadRollupMismatch()andmergeBudgetFields()helpers. - Updated HTTP token tracker to call the shared helpers instead of maintaining local duplicated blocks.
- Updated WebSocket token tracker to call the shared helpers while preserving WS-specific logging fields (e.g.,
transport: 'websocket').
Show a summary per file
| File | Description |
|---|---|
| containers/api-proxy/token-tracker-http.js | Replaces inline warning + budget-field merge blocks with shared helper calls. |
| containers/api-proxy/token-tracker-ws.js | Replaces inline warning + budget-field merge blocks with shared helper calls, preserving WebSocket-specific context fields. |
| containers/api-proxy/token-tracker-shared.js | New shared module implementing the extracted warning + budget-field merge helpers. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 0
🔥 Smoke Test: Copilot PAT Auth — PASS
PR: refactor: extract shared token-tracker budget helpers Overall: PASS
|
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: FAIL
|
Smoke Test ResultsPR: refactor: extract shared token-tracker budget helpers (by @lpcox)
Overall: FAIL — file test could not be verified due to unresolved workflow template variables.
|
🔬 Smoke Test: API Proxy OpenTelemetry Tracing
All scenarios pass. OTEL tracing integration is correctly wired end-to-end.
|
Chroot Version Comparison Results
Overall: ❌ Not all tests passed
|
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.
|
|
@lpcox
|
Smoke Test: Copilot BYOK (Direct Mode) ✅Test Results:
Status: PASS — Running in direct BYOK mode with real credentials held in sidecar. PR Info: Assigned to lpcox, Copilot (
|
Smoke Test: GitHub Actions Services Connectivity
Overall: FAIL
|
|
GitHub API: ✅ PASS Total: PASS
|
…fixes AIC=0) (#5254) * fix(api-proxy): copy token-tracker-shared and otel modules into image PR #4780 (released in v0.27.3) extracted token-tracker-shared.js but did not add it to the Dockerfile COPY list. Because the image copies files individually by name (no bundler), require('./token-tracker-shared') threw MODULE_NOT_FOUND in the container. The graceful-degradation guard in proxy-request.js then silently stubbed trackTokenUsage to a no-op, so the container booted and served traffic normally but never wrote token-usage.jsonl -- causing AI-credit accounting to report 0 for every run since v0.27.3. models.json still appeared because model-discovery.js does not depend on the missing module. otel-exporters.js and otel-serialization.js (required by otel.js) were likewise missing and silently disabled OTEL tracing via its own graceful-degradation guard. Add all three files to the COPY list and add a guard test that computes the require closure from server.js and asserts every local module is present in the Dockerfile, preventing this recurring class of bug. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * fix(test): normalize path.relative() to POSIX separators in dockerfile-copy-coverage test --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Summary
Extracts two duplicated code blocks from
token-tracker-http.jsandtoken-tracker-ws.jsinto a sharedtoken-tracker-shared.jsmodule:warnCacheReadRollupMismatch()— logs warning + diag when observed cache-read tokens don't match normalized rollupmergeBudgetFields()— copies effective token/AI credit budget fields from budgetResult onto the usage recordThis eliminates ~30 lines of duplicated logic while preserving identical behavior (including the WS-specific
transport: 'websocket'field passed via the extra fields parameter).Fixes #4711