Conversation
…vaScript - Add model_multipliers.json to actions/setup/js/ (source of truth for JS) - Update setup.sh to export GH_AW_MODEL_MULTIPLIERS env var via $GITHUB_ENV - Create effective_tokens.cjs with ET computation per the spec (Section 4.3/4.4) - Create effective_tokens.test.cjs with 34 spec-compliant tests (T-ET-001 to T-ET-031) - Update parse_mcp_gateway_log.cjs: ET column in token usage table, ● ET footer line - Export GH_AW_EFFECTIVE_TOKENS via core.exportVariable for use in generated footers - Update messages_footer.cjs: show ● N ET in generated footer when env var available - Update parse_mcp_gateway_log.test.cjs with ET-specific tests Agent-Logs-Url: https://github.com/github/gh-aw/sessions/903d1209-5bf0-420c-b226-645a542280e8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/903d1209-5bf0-420c-b226-645a542280e8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ve_tokens.cjs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/27496c5e-92fe-42e5-bec0-7f9134f64bd8 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in 6934acf — the TypeScript error was |
There was a problem hiding this comment.
Pull request overview
Implements Effective Tokens (ET) computation in the JavaScript actions layer and surfaces ET in both step summaries and generated workflow footers.
Changes:
- Add an ET computation module driven by
GH_AW_MODEL_MULTIPLIERSplus a per-model multipliers JSON source of truth. - Extend MCP gateway token-usage parsing to compute per-model and total ET and display it in the step summary (table column + “● N ET” footer line), exporting
GH_AW_EFFECTIVE_TOKENS. - Update footer generation to append “● N ET” when
GH_AW_EFFECTIVE_TOKENSis available, and expand/adjust test coverage accordingly.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/setup.sh | Exports GH_AW_MODEL_MULTIPLIERS (multiline) to make multipliers available to later JS steps. |
| actions/setup/js/model_multipliers.json | Adds JS-layer source of truth for token class weights + per-model multipliers. |
| actions/setup/js/effective_tokens.cjs | Introduces ET computation (weights + model multiplier with longest-prefix matching). |
| actions/setup/js/effective_tokens.test.cjs | Adds unit tests covering spec cases and edge conditions for ET computation. |
| actions/setup/js/parse_mcp_gateway_log.cjs | Computes and displays ET in token usage summaries; exports GH_AW_EFFECTIVE_TOKENS. |
| actions/setup/js/parse_mcp_gateway_log.test.cjs | Updates/expands tests for ET column and footer behavior. |
| actions/setup/js/messages_footer.cjs | Reads GH_AW_EFFECTIVE_TOKENS and appends “● N ET” to the default footer. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Ensure zero-valued weights fall back to defaults | ||
| const defaults = defaultTokenClassWeights(); | ||
| for (const key of Object.keys(defaults)) { | ||
| if (!weights[key]) { |
There was a problem hiding this comment.
The fallback loop uses a falsy check (if (!weights[key])) which treats an explicit 0 override as “missing” and replaces it with the default. That prevents intentionally disabling a token class via a zero weight; use a null/undefined check (or Number.isFinite) instead so 0 remains a valid override.
| // Ensure zero-valued weights fall back to defaults | |
| const defaults = defaultTokenClassWeights(); | |
| for (const key of Object.keys(defaults)) { | |
| if (!weights[key]) { | |
| // Ensure missing or invalid weights fall back to defaults, but preserve explicit 0 overrides | |
| const defaults = defaultTokenClassWeights(); | |
| for (const key of Object.keys(defaults)) { | |
| const value = weights[key]; | |
| if (value == null || !Number.isFinite(value)) { |
| /** | ||
| * Computes the base weighted token count for a single invocation. | ||
| * | ||
| * Formula (ET specification Section 4.3): | ||
| * base = (w_in × I) + (w_cache × C) + (w_out × O) + (w_reason × R) + (w_cache_write × W) | ||
| * |
There was a problem hiding this comment.
This JSDoc attributes the cache_write term to “ET specification Section 4.3”, but the spec’s base_weighted_tokens formula does not include cache_write (it’s an extension). Update the comment to distinguish spec vs extension to avoid misleading readers.
| function getMultipliersData() { | ||
| if (_parsedMultipliers !== null) { | ||
| return _parsedMultipliers; | ||
| } | ||
|
|
||
| const raw = process.env.GH_AW_MODEL_MULTIPLIERS; | ||
| if (!raw || !raw.trim()) { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
getMultipliersData() only memoizes successful parses; when the env var is missing/invalid JSON it returns null without updating the cache, so repeated calls will repeatedly re-check / re-parse (and re-throw internally for malformed JSON). Consider using a separate “uninitialized” sentinel so you can cache the null result as well.
| const m = getModelMultiplier(model); | ||
| return Math.round(base * m); | ||
| } |
There was a problem hiding this comment.
computeEffectiveTokens() rounds ET to an integer via Math.round. The ET spec defines ET as m × base_weighted_tokens (a real-valued computation) and does not specify rounding; rounding here can lose precision when cached_input weights introduce fractions (e.g., 0.1 × C). Consider returning the exact number and rounding only at presentation boundaries (step summary/footer).
| /** | ||
| * Generates a markdown summary section for token usage data. | ||
| * @param {{totalInputTokens: number, totalOutputTokens: number, totalCacheReadTokens: number, totalCacheWriteTokens: number, totalRequests: number, totalDurationMs: number, cacheEfficiency: number, byModel: Object} | null} summary | ||
| * Includes an Effective Tokens (ET) column per model and a ● ET summary line. | ||
| * @param {{totalInputTokens: number, totalOutputTokens: number, totalCacheReadTokens: number, totalCacheWriteTokens: number, totalRequests: number, totalDurationMs: number, cacheEfficiency: number, totalEffectiveTokens: number, byModel: Object} | null} summary | ||
| * @returns {string} Markdown section, or empty string if no data |
There was a problem hiding this comment.
The ET spec requires disclosing the weights used in reported output when presenting ET. The step summary currently adds an ET column and “● N ET” footer, but doesn’t disclose the token class weights / reference version (even if defaults). Consider adding a short note (e.g., after the table/footer) indicating the weights/multiplier version or reference model used to compute ET.
|
@copilot review comments |
…teway_log.cjs Agent-Logs-Url: https://github.com/github/gh-aw/sessions/24bf8638-6e21-4d13-a457-004b078537f3 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Addressed all 5 review comments in 9245c2f:
Fixed in 9245c2f — same TypeScript error as before (merge commit reverted the previous fix). The Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
…footer Agent-Logs-Url: https://github.com/github/gh-aw/sessions/04f7f3bb-e865-4242-a43d-1979c7a5d662 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done in 1ca3f2f:
Example footer: |
Summary
Implements the Effective Tokens (ET) specification in the action JavaScript layer per the
effective-tokens-specification.md.Changes
New files
actions/setup/js/model_multipliers.json— Source of truth for per-model ET multipliers in the JavaScript actions layer (same data aspkg/cli/data/model_multipliers.json)actions/setup/js/effective_tokens.cjs— ET computation module implementing the spec formula:GH_AW_MODEL_MULTIPLIERSenv var (set bysetup.sh)computeBaseWeightedTokens()— applies per-token-class weightscomputeEffectiveTokens()— returns exact real-valued ET (rounding deferred to presentation)formatET(n)— compact number formatter:<1Kexact,1K–999KasN.NK,≥1MasN.NMGH_AW_MODEL_MULTIPLIERSto avoid redundant re-parsingactions/setup/js/effective_tokens.test.cjs— Spec-compliant tests covering T-ET-001 through T-ET-031 (including the Appendix A worked example) plusformatETtestsModified files
actions/setup/setup.sh— ExportsGH_AW_MODEL_MULTIPLIERSto$GITHUB_ENV(EOF delimiter multiline form) so all subsequent workflow steps have access to the multiplier JSONactions/setup/js/parse_mcp_gateway_log.cjs:parseTokenUsageJsonl()now computeseffectiveTokensper model andtotalEffectiveTokensgenerateTokenUsageSummary()adds an ET column to the per-model table (compact format) and a● Nfooter line (with cache efficiency when available)<sub>ET weights: input=… · …</sub>) after the footer per ET spec requirementswriteStepSummaryWithTokenUsage()exportsGH_AW_EFFECTIVE_TOKENSviacore.exportVariablefor use in generated footersactions/setup/js/parse_mcp_gateway_log.test.cjs— Updated existing column-header test + added ET-specific testsactions/setup/js/messages_footer.cjs:FooterContexttypedef extended with optionaleffectiveTokensfieldgetFooterMessage()appends· ● N(compact format, no "ET" label) to the default footer wheneffectiveTokensis availablegenerateFooterWithMessages()readsGH_AW_EFFECTIVE_TOKENSenv var and passes it aseffectiveTokensin the footer contextHow it works
Example output
Step summary token usage table:
Generated footer in GitHub comments: