Repository navigation
Add friction-cost attribution to audit and usage artifacts - #63662
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot only repo AICs, no usd costs. Remove schema field. |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
ADR required before mergeI did not find an ADR in the PR body, on the PR branch prior to this change, or via a linked issue reference. Evidence reviewed
Action taken
Next action for the author
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes
The new friction-cost pipeline has a blocking producer/consumer type mismatch: latency_ms is emitted as a JSON number from the JS summary generator, but the Go reader and published schemas require an integer.
Blocking theme
pkg/cli/friction_cost.godefineslatency_msasint64, whileactions/setup/js/friction_cost_metrics.cjsforwards rawduration_msvalues from gateway and token-usage data without integer normalization.- Existing gateway activity already models durations as floating-point milliseconds, so a fractional value can make
usage/activity/summary.jsonfail to decode and drop the precomputed friction section fromgh aw logs/gh aw audit.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt54 · 114.6 AIC · ⌖ 7.05 AIC · ⊞ 19.2K
Comment /review to run again
| Tokens FrictionTokens `json:"tokens"` | ||
| Turns int `json:"turns"` | ||
| ToolCalls int `json:"tool_calls"` | ||
| LatencyMS int64 `json:"latency_ms"` |
There was a problem hiding this comment.
This will break friction summary loading as soon as any contributing duration is fractional, because the JS producer emits latency_ms as a generic number while the Go consumer and schema require an integer.
💡 Why this blocks the PR
computeFrictionCost() accumulates duration_ms values from gateway/tool logs and token-usage records without integer normalization, while the existing gateway summary already models those durations as float64. If one input is 12.5, the artifact will contain "latency_ms": 12.5, but FrictionCost.LatencyMS is int64 and the schema says integer, so loadUsageActivitySummary() will fail to unmarshal the precomputed friction section. Please make the type contract consistent end to end: either keep latency as float64 everywhere or round before serialization and keep the integer contract everywhere.
There was a problem hiding this comment.
Fractional gateway and token-usage durations are rounded before aggregation, keeping latency_ms aligned with the Go integer field and schema; regression coverage is in place. Fixed in cc0a155.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Fractional token estimates break schema and Go decoding, while historical logs fallback and advertised contract fields remain incomplete.
Review effort: Balanced
Findings: 1
Open (9)
What changed in this PR
Adds precomputed friction-cost attribution to usage artifacts and exposes it through logs, audits, schemas, and console output.
Changes:
- Computes per-driver friction costs, uncertainty, grouping, and event attribution.
- Propagates friction data through audit/log models and generated schemas.
- Adds tests, reference documentation, and release notes.
| File | Description |
|---|---|
schemas/logs.schema.json |
Adds friction data to logs schema. |
schemas/logs-jsonl.schema.json |
Adds friction data to JSONL records. |
schemas/audit.schema.json |
Adds friction data to audit schema. |
pkg/cli/logs_usage_activity.go |
Loads precomputed friction summaries. |
pkg/cli/logs_run_processor.go |
Preserves friction through processing and caching. |
pkg/cli/logs_report.go |
Exposes friction in reports and tables. |
pkg/cli/logs_orchestrator_filters.go |
Propagates friction into processed runs. |
pkg/cli/logs_models.go |
Extends log data models. |
pkg/cli/logs_cached_json.go |
Restores cached friction data. |
pkg/cli/friction_cost.go |
Defines friction models, formatting, and fallback derivation. |
pkg/cli/friction_cost_test.go |
Tests Go ingestion, derivation, and rendering. |
pkg/cli/audit.go |
Adds friction to audit analysis state. |
pkg/cli/audit_summary_build.go |
Propagates friction into audit summaries. |
pkg/cli/audit_run_pipeline.go |
Restores friction from saved summaries. |
pkg/cli/audit_report.go |
Adds friction to audit output. |
pkg/cli/audit_report_render.go |
Renders friction aggregates and events. |
pkg/cli/audit_analysis_fanout.go |
Loads or derives audit friction data. |
docs/src/content/docs/reference/artifacts.md |
Documents the friction artifact contract. |
actions/setup/js/generate_usage_activity_summary.test.cjs |
Tests summary integration. |
actions/setup/js/generate_usage_activity_summary.cjs |
Writes friction into usage summaries. |
actions/setup/js/friction_cost_metrics.test.cjs |
Tests attribution behavior. |
actions/setup/js/friction_cost_metrics.cjs |
Implements friction attribution. |
.changeset/add-friction-cost-measurement.md |
Records the user-facing feature. |
| groups.push({ | ||
| group_id: groupID, | ||
| primary_source: primarySource, | ||
| event_ids: ranked.map(event => event.id), | ||
| total_occurrences: totalOccurrences, | ||
| counted_occurrences: countedOccurrences, | ||
| suppressed_occurrences: totalOccurrences - countedOccurrences, | ||
| rule: "highest-fidelity source owns overlapping occurrences; lower-fidelity sources contribute only their excess", |
There was a problem hiding this comment.
Group event_ids are now limited to IDs in the emitted event list, and truncated groups are marked explicitly. Fixed in cc0a155 and baa6562.
| for (const dimension of COST_DIMENSIONS) { | ||
| const state = dimensionStates[dimension]; | ||
| if (state !== DIMENSION_UNSUPPORTED && state !== STATE_UNAVAILABLE) { | ||
| dimensionStateSets[dimension].push(state); | ||
| } | ||
| } |
There was a problem hiding this comment.
Counted events with unavailable supported dimensions now contribute unavailable to the aggregate state; unsupported and fully suppressed dimensions remain excluded. Fixed in cc0a155.
| const totalOccurrences = events.reduce((sum, event) => sum + event.occurrences, 0); | ||
| const countedOccurrences = events.reduce((sum, event) => sum + event.counted_occurrences, 0); | ||
| const runAICValues = invocations.map(invocation => invocation.aic).filter(value => value !== null); | ||
| const totalRunAIC = runAICValues.length === invocations.length && invocations.length > 0 ? runAICValues.reduce((sum, value) => sum + Number(value), 0) : null; |
There was a problem hiding this comment.
When malformed token-usage lines are ignored, the run AIC denominator and friction ratio are omitted rather than computed from a partial record set. Fixed in cc0a155.
| const friction = { | ||
| measurement_state: sources.length === 0 ? STATE_UNAVAILABLE : aggregateStates.aic, | ||
| canonical_unit: "aic", |
There was a problem hiding this comment.
The maintainer requirement is repo AIC only: the schema discriminator and USD fields were removed and the schemas/docs updated accordingly. Applied in f334c8f.
| MCPToolUsage: result.MCPToolUsage, | ||
| TokenUsage: result.TokenUsage, | ||
| WorkingSet: result.WorkingSet, | ||
| Friction: result.Friction, |
There was a problem hiding this comment.
The logs path now derives occurrence-only friction for historical runs without a usage summary and preserves precomputed summaries. Fixed in cc0a155.
| "counted_occurrences": 1, | ||
| "suppressed_occurrences": 0, | ||
| "state": "causal", | ||
| "cost": { "aic": 0.5, "tokens": { "total": 128 }, "turns": 0, "tool_calls": 1, "latency_ms": 250 } |
There was a problem hiding this comment.
The driver example now includes every required token class. Fixed in cc0a155.
| "tool_calls": "measured", | ||
| "latency_ms": "measured" | ||
| }, | ||
| "cost": { "aic": 0.5, "tokens": { "total": 128 }, "turns": 0, "tool_calls": 1, "latency_ms": 250 } |
There was a problem hiding this comment.
The event example now includes every required token class. Fixed in cc0a155.
| | `firewall_block` | `network_block` | `firewall` | unsupported | unsupported | unsupported | unsupported | unsupported | | ||
| | `agent_api_error` | `model_error` | `agent_token_usage` | measured | measured | measured | unsupported | measured | | ||
|
|
||
| `measured` means the dimension is read from the friction record itself. `derived` means the dimension is attributed causally when a follow-up invocation can be linked, and statistically otherwise. `unsupported` means no data source expresses that dimension for the driver: firewall blocks are counted as occurrences, but their cost is never estimated, because a blocked request leaves no invocation-level trace to attribute. Drivers whose source is absent, or that produced no occurrences, are listed in `unmeasured_drivers` with a `no_occurrences` or `source_unavailable:<source>` reason. |
There was a problem hiding this comment.
The firewall driver matrix and prose now reflect the statistically derived dimensions supported by the implementation. Fixed in cc0a155.
|
|
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /grill-with-docs — this is a well-engineered, thoroughly tested feature (825 lines of tests across JS/Go for ~1300 lines of implementation). Two actionable findings, both non-blocking.
📋 Key Themes & Highlights
Key Themes
- Docs/code drift on
firewall_block: the driver-matrix table states firewall cost isunsupported, but the code (and its own tests) statistically estimate it — this will mislead readers ofartifacts.mdabout whatunmeasured_drivers/dimension_statesto expect for firewall blocks. - All-or-nothing
friction_ratiogate: a single invocation missingai_credits_this_responsesilently drops the entire run'sfriction_ratio/total_run_aic, rather than degrading gracefully with a partial-data indicator.
Positive Highlights
- ✅ Deterministic causal grouping with clear fidelity ranking prevents double-counting cleanly, and is well covered by dedicated tests.
- ✅ Attribution states (
measured/causal/statistical/unavailable) are consistently modeled across the JS computation, Go structs, schemas, and docs, with explicit backward-compatibility tests for pre-friction historical runs. - ✅
deriveFrictionFromLogscorrectly never fabricates AIC for historical runs and is markedDerived: trueso it can never silently overwrite precomputed data.
@copilot please address the review comments above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 166.8 AIC · ⌖ 14.5 AIC · ⊞ 10K
Comment /matt to run again
| | `mcp_tool_error` | `tool_failure` | `mcp_gateway` | derived | derived | unsupported | measured | measured | | ||
| | `session_tool_failure` | `tool_failure` | `agent_session` | derived | derived | unsupported | measured | unsupported | | ||
| | `integrity_filter` | `integrity_filter` | `mcp_gateway` | derived | derived | unsupported | measured | unsupported | | ||
| | `firewall_block` | `network_block` | `firewall` | unsupported | unsupported | unsupported | unsupported | unsupported | |
There was a problem hiding this comment.
[/grill-with-docs] Docs say firewall_block AIC/Tokens are unsupported (line 441 "firewall blocks are counted as occurrences, but their cost is never estimated"), but FRICTION_DRIVERS.firewall_block.dimensions (line 87) declares aic: "derived", tokens: "derived" — and the test suite confirms firewall AIC is statistically estimated when invocation evidence exists (friction_cost_metrics.test.cjs:275-282).
💡 Why this matters
The driver-matrix table is meant to be the authoritative reference for what unmeasured_drivers reasons and dimension_states values a consumer should expect. As written, a reader of the docs would incorrectly assume firewall-block cost is never attributed, when in practice it is statistically apportioned exactly like mcp_tool_error/session_tool_failure/integrity_filter. Either the docs row should read derived | derived | unsupported | unsupported | unsupported, or the code comment "a blocked request leaves no invocation-level trace to attribute" needs to be removed/reworded since it directly contradicts the statistical-apportionment test.
@copilot please address this.
There was a problem hiding this comment.
The firewall driver matrix and prose now reflect the statistically derived dimensions supported by the implementation. Fixed in cc0a155.
| const totalOccurrences = events.reduce((sum, event) => sum + event.occurrences, 0); | ||
| const countedOccurrences = events.reduce((sum, event) => sum + event.counted_occurrences, 0); | ||
| const runAICValues = invocations.map(invocation => invocation.aic).filter(value => value !== null); | ||
| const totalRunAIC = runAICValues.length === invocations.length && invocations.length > 0 ? runAICValues.reduce((sum, value) => sum + Number(value), 0) : null; |
There was a problem hiding this comment.
[/codebase-design] total_run_aic/friction_ratio are only emitted when every invocation in the run has a non-null aic field (runAICValues.length === invocations.length). If even one invocation is missing ai_credits_this_response, the whole run's friction_ratio silently disappears rather than being computed from the subset that does have data.
💡 Suggested improvement
This all-or-nothing gate means friction_ratio — the headline "how much of this run was wasted" metric documented in artifacts.md — will be entirely absent on any run with a single partially-instrumented invocation, with no distinction from a run with zero AIC data at all (both just omit the field). Consider tracking runAICValues.length as totalRunAIC and separately reporting whether it's a complete or partial denominator (e.g. via an is_partial flag or reusing the existing uncertainty-state pattern), so a single missing sample doesn't erase an otherwise-computable ratio, and so consumers can't confuse "no data" with "one small gap."
@copilot please address this.
There was a problem hiding this comment.
Run totals use the known AIC records and set total_run_aic_partial when valid records omit AIC; if malformed records were ignored, the total remains unknown. Fixed in 87be32a.
There was a problem hiding this comment.
Impeccable review (audit + harden mode — new attribution/measurement feature)
Reviewed the new friction-cost attribution pipeline (friction_cost_metrics.cjs, friction_cost.go) end-to-end, including the causal-grouping/dedup logic, statistical apportionment, and Go wiring into usage/audit/logs. Go unit tests (TestFriction*, TestDeriveFriction*) pass locally. JS test dependencies couldn't be installed in this sandbox (no network access to npm registry), so I exercised the compiled module directly with reproduction scripts instead.
Found 1 blocking issue (inline): the top-level measurement_state / dimension_states aggregation drops unavailable per-event contributions before folding, so a run can report measurement_state: "measured" while a driver with real, known-unattributed cost (e.g. firewall_block with no baseline invocations) silently contributes nothing to the aggregate. Since this artifact is designed to be read standalone (without downloading raw logs, per the PR description), this directly undermines the core trust claim of the section — reproduction included in the inline comment.
No other high-signal issues found in the causal-grouping, invocation-linkage, or Go rendering/wiring code on this pass.
@copilot please address the inline comment before merge.
Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
codeload.github.com
To allow these domains, add them to the network.allowed list in your workflow frontmatter:
network:
allowed:
- defaults
- "codeload.github.com"See Network Configuration for more information.
🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · copilot · sonnet50 · 145 AIC · ⌖ 13.3 AIC · ⊞ 8.1K
|
|
||
| for (const dimension of COST_DIMENSIONS) { | ||
| const state = dimensionStates[dimension]; | ||
| if (state !== DIMENSION_UNSUPPORTED && state !== STATE_UNAVAILABLE) { |
There was a problem hiding this comment.
Blocking: aggregate measurement_state/dimension_states silently ignore unavailable per-event states
This filter drops both unavailable and unsupported event-level states before folding them into the aggregate via weakerState. That means an unavailable contribution (data was expected but couldn't be attributed) is treated the same as a driver that never produced any events for that dimension — it simply doesn't count toward the aggregate at all.
Concretely: if a run has one agent_api_error event with aic: measured and one firewall_block driver whose AIC is unavailable (no baseline invocations to statistically apportion its cost), the top-level friction.measurement_state reports "measured" even though 3 firewall-block occurrences contributed zero attributed cost. Downstream consumers reading only the usage artifact (per the PR's stated goal) will see a confidently "measured" friction total that silently omits real, known-unattributed cost.
Reproduced locally:
const { computeFrictionCost } = require("./friction_cost_metrics.cjs");
const tokenUsageContent = JSON.stringify({ timestamp: "2026-01-01T00:00:01Z", input_tokens: 10, output_tokens: 5, ai_credits_this_response: 1.0, error: "boom" });
const firewall = { requests_by_domain: { "blocked.example": { blocked: 3 } } };
const { friction } = computeFrictionCost({ tokenUsageContent, tokenUsageAvailable: true, firewall });
console.log(friction.measurement_state); // "measured" — but firewall_block driver.state is "unavailable"Suggested fix: only exclude unsupported from the aggregate fold, and let unavailable participate (it already ranks lowest in STATE_RANK, so including it will correctly downgrade the aggregate to unavailable/statistical when any contributing driver couldn't attribute cost):
if (state !== DIMENSION_UNSUPPORTED) {
dimensionStateSets[dimension].push(state);
}@copilot please address this.
There was a problem hiding this comment.
Counted events with unavailable supported dimensions now contribute unavailable to the top-level measurement-state rollup; unsupported and fully suppressed dimensions remain excluded. Fixed in cc0a155.
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
|
|
|
@copilot merge main and fix conflicts |
The aggregate now includes unavailable states from counted events, with coverage in
The integer-contract fixes were already present at |
|
@copilot resolve the merge conflicts on this branch. |
|
🎉 Great work on the friction-cost attribution feature! This PR adds comprehensive measurement and reporting of execution costs across audit and usage artifacts, including: ✅ Complete implementation — Attribution model with token classes, turns, tool calls, latency, and USD equivalents The PR looks ready for core team review. Nice work!
|
…-agentic-workflow # Conflicts: # actions/setup/js/generate_usage_activity_summary.test.cjs # pkg/cli/audit_analysis_fanout.go # pkg/cli/logs_usage_activity.go Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com>
The merge conflicts were resolved and committed in
|
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in |
|
🎉 This pull request is included in a new release. Release: |
Introduce friction cost as the estimated avoidable marginal execution cost, measured canonically in AIC. Precompute attribution during workflow conclusion so audit and fleet analysis can use the compact
usageartifact without downloading raw logs.Attribution model
Usage artifact
friction-cost/v1section toactivity/summary.json.Audit and logs
usage.Documentation and schemas
{ "friction": { "schema": "friction-cost/v1", "measurement_state": "causal", "cost": { "aic": 4.2 }, "estimated_usd": 0.042, "friction_ratio": 0.08, "uncertainty": { "aic": { "lower_bound": 0, "upper_bound": 4.2, "confidence": "medium" } } } }Run: https://github.com/github/gh-aw/actions/runs/36265507090
pr-sous-chefBranch refresh requested by PR Sous Chef run https://github.com/github/gh-aw/actions/runs/36269774156