Skip to content

feat(proxy): attribution labels for savings, stats and export - #4699

Open
mosatch wants to merge 16 commits into
headroomlabs-ai:mainfrom
mosatch:feat/attribution-labels
Open

mosatch wants to merge 16 commits into
headroomlabs-ai:mainfrom
mosatch:feat/attribution-labels

Conversation

@mosatch

@mosatch mosatch commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

Description

Every figure the proxy reports is a sum over requests, and today only project can split it. Each extra split would need its own header, context variable, outcome field, tracker map, ledger key and dashboard table. This PR adds those pieces once, generically, as proposed in #4676. A request carries a {dimension: label} mapping, and every surface that splits by project can now split by any dimension.

  • headroom/proxy/attribution.py: a dimension is a name plus a resolver. Built-ins are project (existing header and /p/<name> prefix), client (X-Client, else the User-Agent) and tenant (the tenant-key header). Middleware resolves the enabled dimensions once per request into a context variable. HEADROOM_ATTRIBUTION_DIMENSIONS picks which ones (default, -name, none, labels). A name with no resolver is read from X-Headroom-<name>, so team or cost-center needs no code. X-Headroom-Labels: team=infra; cost-center=42 sends several at once. With the labels token a client may name any dimension that way, including a built-in that is not listed; a listed resolver always has the last word, and -name keeps a name out. register_dimension() lets an extension or a deployment's identity layer add its own, and replace=True takes over a built-in. An extension's dimension is reserved from labels unless it registers with client_settable=True.
  • Outcome funnel: emit_request_outcome merges the middleware labels, any the handler knew, and the existing project / client fields. The existing fields win their own dimension only while it is enabled and still resolved by its built-in: a dimension turned off stays out, and a replaced resolver keeps the label it gave. It hands the result to the savings tracker, the ledger, the request log tags and OutcomeSnapshot.attribution (PR 1).
  • Savings tracker: a bounded dimensions map per label, with the same accounting and eviction as projects: 16 dimensions, 50 labels each, the smallest and oldest label evicted first. project is booked there like any other dimension. projects is unchanged, so existing readers keep working.
  • Savings ledger and CLI: events record their labels as dims. headroom savings --where dim=label filters every window, and --by <dim> splits the lookback window. An event with no label for a dimension shows as unknown. A dimension named like a report figure (calls, tokens_saved, …) cannot be grouped by and --by refuses it.
  • /stats, /stats-lifetime, dashboard: by_dimension totals and a Savings by Label table. Labels can name machines and people, so these fields (savings.by_dimension, persistent_savings.by_dimension and /stats-lifetime by_dimension) are returned only to callers that _request_can_view_stats_metadata admits: the same gate as recent_requests, including proxy-token holders since feat(proxy): show per-request /stats metadata to token-authenticated callers #4168. security-model.mdx lists the new fields in its token table.
  • OpenTelemetry: HEADROOM_OTEL_ATTRIBUTION=project,team (or all) adds labels as datapoint attributes. Off by default, since every label value becomes a series.

Labels are self-reported. They attribute usage and authorize nothing. Authenticated identity stays behind set_identity_resolver.

This is PR 2 of the split proposed in #4676. It builds on #4698, which has merged; the branch is rebased onto main and carries only this PR's commits. host and traffic_class follow in PR 3.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Performance improvement
  • Code refactoring (no functional changes)

Changes Made

  • headroom/proxy/attribution.py, attribution_builtins.py (new): dimensions, resolvers, sanitizing, context, OTel provider.
  • headroom/proxy/server.py: resolve labels where project attribution is already captured (HTTP and WebSocket); by_dimension in /stats and /stats-lifetime behind the existing metadata gate.
  • headroom/proxy/outcome.py, handlers/openai.py, prometheus_metrics.py: carry the labels to the tracker and ledger.
  • headroom/proxy/savings_tracker.py: per-dimension maps, persisted and normalized on load.
  • headroom/savings_ledger.py, headroom/cli/savings.py: dims on events; --by, --where.
  • headroom/dashboard/templates/dashboard.html: Savings by Label.
  • headroom/pipeline.py: the OutcomeSnapshot.attribution comment now points to the new module.
  • Docs: docs/content/docs/attribution.mdx (new), meta.json, and security-model.mdx (token table).
  • Tests: tests/test_attribution.py, tests/test_attribution_savings.py.

Testing

  • Unit tests pass (pytest)
  • Linting passes (ruff check .)
  • Type checking passes (mypy headroom)
  • New tests added for new functionality
  • Manual testing performed

Test Output

$ pytest tests/test_proxy tests/test_pipeline.py tests/test_pipeline_outcome_stage.py \
    tests/test_5xx_accounting_all_providers.py tests/test_savings_tracker_output_cost.py \
    tests/test_savings_tracker_zero_price.py tests/test_dashboard_token_savings.py \
    tests/test_prometheus_obs_counters.py tests/test_attribution.py tests/test_attribution_savings.py
715 passed, 5 failed                      # branch base: upstream/main 3c66c91e
# The 5 failures (test_copilot_nes_routing.py: 1, test_openai_transport_path_prefix.py: 4) fail the same
# way on upstream/main 3c66c91e without this change, in my local test image.

$ pytest tests/test_attribution.py tests/test_attribution_savings.py     # this head
72 passed
$ pytest tests/test_attribution.py tests/test_attribution_savings.py     # same tests, code at the previous head
14 failed, 58 passed

$ ruff check <changed files> && ruff format --check <changed files>     # ruff 0.17.3
All checks passed!
12 files already formatted

$ mypy headroom
No errors in the changed files. The errors reported are in memory/adapters/embedders.py
and providers/model_metadata.py, identical on upstream/main, from local library versions.

Real Behavior Proof

  • Environment: macOS, a Headroom proxy serving as a LAN gateway for several machines (Claude Code and scripts).
  • Exact command / steps: a superset of this change (all of RFC: one label set for request attribution (project, client, host, …) instead of one axis per dimension #4676) has run there since 2026-11-06. headroom savings --by client over the durable ledger.
  • Observed result: headroom savings --by client splits the 30-day window by harness:
    Savings by client:
      claude-code  $861.1609  88,380 calls · 425,817,184 tokens (3.7%)
      proxy        $32.0160  1,606 calls · 10,301,986 tokens (43.6%)
      claude       $1.5449  151 calls · 735,884 tokens (41.2%)
    
  • Not tested: this exact subset was not deployed on its own. It is covered by the tests above, cut from 3c66c91.

Runtime Rollout Safety

  • Rollout-managed feature(s): none
  • Minimum rollout channel: n/a
  • Stable/default behavior changed: additive. The default dimensions (project, client, tenant) are resolved and booked, and /stats and /stats-lifetime gain by_dimension for metadata-admitted callers only. Existing fields and projects are unchanged. The OTel attributes and the labels token are off by default.
  • Kill switch / disable path: HEADROOM_ATTRIBUTION_DIMENSIONS=none
  • Unsafe override required: no
  • Qualification impact: none expected; resolving labels is a few header reads per request, and every step is wrapped so a failure leaves the request unattributed.
  • Rollback path: revert. Persisted dimensions state is ignored by older versions.

Review Readiness

  • I have performed a self-review
  • This PR is ready for human review

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I did not edit CHANGELOG.md — it is generated by release-please from my Conventional Commit PR title (a CI guard enforces this)

Screenshots (if applicable)

N/A

Additional Notes

  • Review follow-up (replacement resolvers, open labels):
    • A resolver registered with replace=True for client or project is no longer overridden at the outcome funnel by the handler's own RequestOutcome.client / project, nor by the WebSocket and Codex handlers binding those labels. Its label reaches by_dimension, the ledger's dims, the request-log tags and OutcomeSnapshot.attribution. test_a_replaced_resolver_is_not_overridden_by_the_legacy_field takes the labels the middleware bound on a real request and checks /stats, /stats-lifetime, the log tags and the ledger event. Legacy bookkeeping is separate and unchanged: the projects totals and the ledger event's own client key (which headroom savings --by client reads) still record what the proxy detected.
    • by_dimension.project is now booked from the request's labels like every other dimension. It used to be served from projects, which could not show a replaced or client-named project and ignored -project. It therefore starts counting from this version; projects keeps its history.
    • labels accepts a registered dimension that is not selected (resolve_attribution(..., ['labels']) with project=web returns it). A selected resolver keeps precedence over the header, even when it resolves nothing, and -name is never re-admitted, by a client or by bind_label.
    • That is limited to client-settable dimensions. The built-ins are; a dimension an extension registers is reserved unless it passes client_settable=True, so a label taken from authenticated identity cannot be supplied in a header when the operator has not listed it.
    • unregister_dimension restores a replaced built-in. default, none and labels are rejected as dimension names. X-Headroom-Labels is filtered before the 16-label cap applies.
  • Second review follow-up:
    • aggregate_savings refuses a group_by name that is also a row field (ROW_FIELDS), since {<dimension>: <label>, **figures} cannot hold both; the CLI reports it as a --by error. --where on such a name still works.
    • The ledger guarantee is narrowed in the docs: an event's client key is always the harness and dims never carries client; every other dimension is written as resolved.
    • /stats no longer returns persistent_savings.by_dimension to callers outside the metadata gate (it stripped only savings.by_dimension).
    • Labels a handler supplies on RequestOutcome.attribution obey the operator's switches. A header label is percent-decoded once. A new dimension can evict the smallest idle one; merged labels are capped at 16; only the first 4 KB of X-Headroom-Labels is read. default, none and labels cannot become dimensions by any path.
    • Request-log client / project tags follow the label only for a dimension the operator listed.
    • headroom savings: a name that is not a dimension name, or a repeated --where dimension, is an error; a filter that matches nothing says so; --by client prints one client section.
    • Not covered: no single test drives a model request from middleware to outcome through a handler; the proxy tests pass the labels the middleware bound to emit_request_outcome.
  • projects is kept alongside by_dimension.project so no existing reader breaks. Folding it in can be a later change.
  • How this relates to request tags: extract_tags already turns every X-Headroom-* header into a request-log tag. Tags stay as they are, per request, unbounded and never aggregated. A dimension is an operator-chosen, capped subset of those same headers that gets booked into the tracker and ledger. X-Headroom-Team: infra is therefore both a tag and, once team is listed, a label, under the same credential rule (is_credential_header).
  • Labels and dimension names are sanitized and length-capped. A header resolver is never built for a header is_credential_header already treats as a credential (authorization, cookie, API-key headers, the proxy token), so a dimension name cannot turn a secret into a persisted label.

@github-actions

github-actions Bot commented Nov 8, 2026 •

Copy link
Copy Markdown
Contributor

PR governance

This PR follows the template and is marked ready for human review.

@github-actions github-actions Bot added the status: needs author action Pull request body or readiness checklist still needs author updates label Nov 8, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 5 potential issues.

Devin Review

Comment thread headroom/cli/savings.py Outdated
report = savings_ledger.aggregate_savings(
retention_days=days, group_by=group_by, where=where_filter
)
routing = _proxy_routing()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Filtered savings include global routing totals

With --where host=studio, savings prints studio's ledger totals beside unfiltered routing totals. _proxy_routing reads the proxy-wide rollup, so routing savings appear attributed to studio.

Learn more

The ledger applies where_filter before computing any savings totals, but _proxy_routing reads /stats.routing without a corresponding label filter. The CLI appends that global rollup to both JSON and human-readable filtered output, even when the selected label has no matching ledger events.

Example: A studio request saved $1 and another host's routed requests saved $20. headroom savings --where host=studio displays studio's $1 ledger result alongside $20 in model routing, although those routing requests were not from studio.

Recommended fix: Suppress routing when a --where filter is active, unless the routing rollup gains a compatible per-label filter. Keep unfiltered routing output unchanged.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread headroom/proxy/outcome.py Outdated
Comment on lines +635 to +636
{"project": project} if project else None,
{"client": outcome.client} if outcome.client else None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Disabled or replaced labels still get recorded

When client is disabled or given a custom resolver, emit_request_outcome records the handler's client instead. The legacy fields override resolved labels, so configured exclusions and custom identities cannot control those dimensions.

Learn more

Request middleware resolves only the configured dimensions and stores the results in the request context. The outcome funnel then adds outcome.project and outcome.client unconditionally to that mapping. Consequently the dimension configuration cannot remove those names, and a registered resolver for either name loses to the handler's legacy field. The project also remains in the existing project map for backward compatibility; the new by_dimension map needs to distinguish that compatibility behavior from enabled attribution.

Example: With HEADROOM_ATTRIBUTION_DIMENSIONS=default,-client, a request from Codex has no middleware client label. Its RequestOutcome.client='codex' adds client=codex anyway, and /stats-lifetime.by_dimension.client reports it.

Recommended fix: Gate adding legacy project and client to the new attribution mapping on enabled dimensions, and do not overwrite values supplied by a registered resolver. Preserve the existing legacy project and client bookkeeping separately.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread headroom/proxy/attribution.py Outdated
for name, label in parse_labels(request.header(LABELS_HEADER)).items():
if len(labels) >= MAX_DIMENSIONS:
break
if name not in labels and name not in registry and not _is_credential(name):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Open labels omit registered dimensions

With only labels enabled, resolve_attribution drops combined-header labels whose names are registered dimensions. The registry check excludes names even when their resolver was not selected, leaving those requests unattributed.

Learn more

The labels configuration token reads client-named dimensions from X-Headroom-Labels. A registered dimension that is not in names has not run its resolver, but the open-label loop still discards its supplied value solely because it exists in registry.

Example: An extension registers team with default=False; the operator sets HEADROOM_ATTRIBUTION_DIMENSIONS=labels, and a client sends X-Headroom-Labels: team=infra. resolve_attribution returns no team label rather than team=infra.

Recommended fix: Let open labels supply registered names that were not resolved through the selected dimensions, while preserving the precedence of explicitly selected resolvers and credential checks.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread headroom/proxy/attribution.py Outdated

def format_labels(labels: Mapping[str, str]) -> str:
"""Inverse of :func:`parse_labels`, for building ``X-Headroom-Labels``."""
return "; ".join(f"{k}={v}" for k, v in normalize_attribution(labels).items())

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Label formatting loses values with separators

When a label contains a comma or semicolon, format_labels emits an ambiguous header value. parse_labels splits it, so a round trip changes the label.

Learn more

The new formatter advertises an inverse of parse_labels, which separates pairs on commas and semicolons. Label sanitation permits both characters, and the formatter does not escape them. A client using this exported helper therefore cannot faithfully send some valid labels through the combined header.

Example: format_labels({'team': 'design, ops'}) emits team=design, ops. Parsing that value yields {'team': 'design'} instead of the original label.

Recommended fix: Define an escaping convention shared by format_labels and parse_labels, or reject separator characters from labels before formatting and document that restriction.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +1647 to +1649
No labelled requests yet. <span class="font-mono">headroom wrap</span> sends
<span class="font-mono">X-Headroom-Host</span>; add your own with
<span class="font-mono">HEADROOM_LABELS="team=infra"</span>.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Dashboard empty state promises host labels

The empty state says headroom wrap sends X-Headroom-Host, but host support is described as a later change. Check whether this instruction works in the merged release.

Devin Review


Was this helpful? React with 👍 or 👎 to provide feedback.

@codecov-commenter

codecov-commenter commented Nov 8, 2026 •

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 94.28571% with 34 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
headroom/proxy/attribution.py 95.97% 6 Missing and 7 partials ⚠️
headroom/proxy/savings_tracker.py 91.76% 3 Missing and 4 partials ⚠️
headroom/cli/savings.py 88.88% 4 Missing and 2 partials ⚠️
headroom/proxy/server.py 75.00% 6 Missing ⚠️
headroom/proxy/gateway_turn.py 84.61% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: needs author action Pull request body or readiness checklist still needs author updates labels Nov 8, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The follow-up at 5ad4dc944c83b71a9cf55d376cbf58d9f8390658 addresses the filtered-routing totals and disabled-client booking findings. Two remaining configuration behaviors need resolution before approval:

  1. A replacement resolver is lost at the outcome funnel. Registering client with register_dimension(..., replace=True) and resolving it produces custom-client. Passing that resolved attribution into a real proxy outcome with legacy RequestOutcome.client='codex' then records only codex in /stats.savings.by_dimension.client. emit_request_outcome unconditionally overlays the enabled legacy client/project fields after resolved labels. Preserve an explicitly resolved replacement value in the new attribution map, keeping legacy bookkeeping separate, or explicitly reject/document replacement for these reserved dimensions. Please cover the end-to-end outcome/stats behavior rather than only resolver output.

  2. The open-label contract excludes registered but unselected names. resolve_attribution(AttributionRequest(headers={'x-headroom-labels':'project=web'}), ['labels']) returns {}. The documentation says labels accepts any dimension a client names; the registry exclusion also applies when that registered resolver was not selected. Distinguish selected resolvers from registered-but-unselected names, or explicitly document and test those names as reserved. A selected resolver should still retain precedence over a client header.

Both cases reproduce on the exact published head in owned probes. The source includes current main. These are attribution correctness/contract issues, not CI failures. No contributor logic was rewritten; the broader 18-file feature is not claimed fully qualified by these probes. Maintainers own CI; no contributor CI action requested. Nothing merged.

@github-actions github-actions Bot added status: needs rebase Pull request branch is behind the base branch on files it also changes status: has conflicts Pull request has merge conflicts with the base branch and removed status: ready for review Pull request body is complete and the author marked it ready for human review labels Nov 10, 2026
@mosatch
mosatch force-pushed the feat/attribution-labels branch from 5ad4dc9 to eac9e69 Compare November 11, 2026 20:28
@mosatch

mosatch commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

@JerrettDavis thanks for the review. Both points are fixed in the latest commit, and the branch is rebased now that #4698 has merged (it no longer carries that PR's commits).

  1. Replacement resolver at the outcome funnel. emit_request_outcome now overlays the legacy client / project fields only while the dimension is enabled and still resolved by its built-in, so a replace=True resolver's label is what reaches by_dimension, the ledger's dims, the request-log tags and OutcomeSnapshot.attribution. The WebSocket and Codex handlers that bind those two labels follow the same rule. Legacy bookkeeping stays separate: the projects totals and the ledger event's own client key still record what the proxy detected. test_a_replaced_resolver_is_not_overridden_by_the_legacy_field replaces both client and project, takes the labels the middleware bound on a real request, and checks /stats, /stats-lifetime, the log tags and the ledger event.

    Getting project right needed one more change: by_dimension.project was served from the projects map, so it could not show a replaced or client-named project, and it ignored -project. project is now booked like every other dimension. projects itself is unchanged.

  2. Open labels and registered names. labels now accepts a registered dimension whose resolver was not selected, so resolve_attribution(..., ['labels']) with project=web returns it, and it reaches /stats. A selected resolver keeps precedence over the header, including when it resolves nothing, and a name removed with -name is never re-admitted, by a client or by a handler.

    I did not open this to every registered name. A dimension an extension registers may come from authenticated identity, and a client should not be able to supply that one through a header just because the operator left it off the list. register_dimension therefore takes client_settable (default off): the built-ins, which are self-reported already, are client-settable, and an extension's dimension stays reserved unless it opts in. Documented and tested both ways. If you would rather have all registered names reserved, or all open, that is a small change.

Smaller things fixed along the way: unregister_dimension restores a built-in that had been replaced; default, none and labels are rejected as dimension names; and X-Headroom-Labels is filtered before the 16-label cap is applied, so names that are dropped no longer crowd out valid ones.

@JerrettDavis

Copy link
Copy Markdown
Collaborator

Two new review points on the current head: (1) aggregate_savings(..., group_by=["calls"]) loses a calls=infra label because _ranked merges the numeric calls field over it; please choose a collision-free row representation or reject conflicting dimension names and cover this case. (2) Please reconcile the replacement-client ledger guarantee: docs promise the resolved label in dims, but _event_dims drops it and the new test asserts that absence. Keeping the legacy client key is understood; the question is whether the resolved label is also retained or the guarantee narrowed. Isolated probes in /job/output/review_probes.py reproduce both; full pytest could not start because pytest is absent. No live proxy behavior claimed.

@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: has conflicts Pull request has merge conflicts with the base branch status: ready for review Pull request body is complete and the author marked it ready for human review labels Nov 12, 2026
@mosatch

mosatch commented Nov 12, 2026

Copy link
Copy Markdown
Contributor Author

@JerrettDavis thanks, both are addressed in the new commit (a plain fast-forward on top of the head you reviewed). It also contains a set of other fixes, all listed below so nothing in the diff is a surprise.

Your two points

  1. group_by=["calls"]. A row is {<dimension>: <label>, **figures}, so a dimension named like a figure cannot be grouped by. I chose to reject rather than change the row shape, which headroom savings --json consumers would see: aggregate_savings raises ValueError for any group_by name in ROW_FIELDS (calls, tokens_saved, cost_usd, …), and the CLI turns that into a --by usage error. The label is still recorded and --where calls=infra still filters. Tests: test_ledger_refuses_to_group_by_a_name_that_is_a_row_field (every field name), test_savings_cli_rejects_by_a_row_field.

  2. Replacement client and the ledger. I narrowed the guarantee rather than widen the ledger. An event has one client key, always the harness, and a client label is never written to dims; the docs now say exactly that, and that --by client / --where client=… report the harness even when the resolver is replaced. Every other dimension, a replaced project included, is written to dims as resolved. Keeping a second client under dims would have given --by client and --where client= two possible meanings.

Everything else in the commit

Prompted by those two, I went through the rest of the contract the same way and fixed what did not hold.

Access

  • /stats stripped savings.by_dimension for callers outside the metadata gate but still returned it under persistent_savings. Both are stripped now. test_network_callers_do_not_see_labels asserts the label string is absent from the whole /stats and /stats-lifetime bodies.

Operator switches

  • Labels a handler supplies on RequestOutcome.attribution were booked even for a dimension that is off or removed with -name. They now obey the same switches as bind_label (test_a_handler_cannot_book_a_dimension_that_is_off).
  • default, none and labels could still become dimensions through bind_label, a merge, or an explicit dimensions= list. They cannot now (test_spec_words_cannot_be_bound_or_merged_as_dimensions).
  • Request-log client / project tags stay what the proxy detected unless the operator listed the dimension, so a label a client merely names through labels does not rewrite them (test_client_named_labels_do_not_change_the_request_log_tags).

Label handling

  • A label was percent-decoded again at each stage, so a%252541 was stored differently by the context, the snapshot and the tracker. It is decoded once, where the header is read (test_a_header_label_is_percent_decoded_exactly_once). One contract change follows from that: resolve_attribution no longer decodes what a resolver returns, so a resolver that reads a header itself decides whether to decode it. sanitize_label gained decode= for this. The built-ins are unaffected.
  • Only the first 4 KB of X-Headroom-Labels is read, since it is parsed on every request (test_only_the_start_of_the_labels_header_is_read).

Bounds

  • The tracker's 16 dimension slots were first come, first kept, for good. A new dimension now evicts the smallest, oldest idle one, the same rule as labels (test_a_new_dimension_evicts_the_smallest_idle_one).
  • Merged labels are capped at 16, with later layers (what a handler knows) keeping their place (test_merge_keeps_the_later_layers_when_over_the_cap). bind_label does not add a 17th (test_bind_label_stops_at_the_cap).

aggregate_savings and headroom savings

  • A --where or --by name that is not a usable dimension name was dropped silently, giving an unfiltered report. It is now a ValueError / usage error (test_ledger_rejects_names_that_are_not_dimensions).
  • group_by given as a bare string is one name, not its characters.
  • A --where label is cleaned the same way stored labels are before comparing.
  • SavingsReport.dimensions lists every dimension in the retained events, as its comment says; it was computed after the filter (test_ledger_lists_every_dimension_whatever_the_filter).
  • Repeating --where on one dimension is an error (it kept only the last); a filter that matches nothing says so and names the filter; --by client prints one client section, not two (test_savings_cli_filter_edges).
  • --by help no longer names host and traffic_class, which arrive in the next PR.

Smaller

  • Built-in resolvers are installed under a lock and the flag is set afterwards, so a concurrent first request cannot see a partial registry.
  • The dashboard table sorts rows by tokens saved itself; integer-like labels such as 42 otherwise jumped to the top.
  • test_a_bad_spec_warns_once_not_per_request asserted <= 1 warnings and could pass with none; it now clears the cache and asserts exactly one.

Docs (attribution.mdx, security-model.mdx)

  • --where applies to every window; --by splits the lookback window only.
  • The 16 / 50 caps are the proxy's lifetime totals; the ledger keeps each event's labels for its retention window.
  • none stops labels being resolved and booked; the ledger's own client key is still written.
  • unknown covers any event with no label for the dimension, not only old ones.
  • -name removes from what the rest of the list selects, so -client alone selects nothing; write default,-client.
  • The gate covers persistent_savings.by_dimension too, and /stats-history carries by_dimension behind its own operator-only access.
  • project has its own dashboard table, read from the older projects totals.

Not covered

The proxy tests hand the labels the middleware bound to emit_request_outcome directly, so there is no single test that drives a model request from middleware to outcome through a handler.

@JerrettDavis

Copy link
Copy Markdown
Collaborator

A remaining formatter case changes attribution labels: from headroom.proxy.attribution import format_labels, parse_labels; print(parse_labels(format_labels({'team': 'release%41'}))) prints {'team': 'releaseA'}. Expected the valid literal label release%41 to survive the advertised inverse operation; actual behavior silently puts it in a different bucket because parsing percent-decodes it. The separator rejection fix does not cover percent escapes. Please choose and test an explicit formatter policy (encode literal percent signs, or reject/document such values). This was reproduced in an isolated Python unit probe; no live proxy claim.

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed; approval remains pending the items below.

Reviewed head d13f4b925bcc59b1a94a95b84cb8819a107fc932 against main c05fbdddc873078dd97cb9ede841e75ef7ed0bef.

Minor — headroom/proxy/attribution.py:417

format_labels silently changes valid labels containing percent escapes when its output is parsed. Choose an explicit encoding or rejection policy and add a round-trip regression test.

Qualification still required:

  • Passing test evidence is missing
  • Exact-source provenance review is incomplete
  • Check/run 131027444949 is not successful or baseline-qualified
  • Check/run 131027444646 is not successful or baseline-qualified
  • Check/run 43172051543 is not successful or baseline-qualified

@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: ready for review Pull request body is complete and the author marked it ready for human review labels Nov 13, 2026
@mosatch

mosatch commented Nov 13, 2026

Copy link
Copy Markdown
Contributor Author

@JerrettDavis thanks, fixed in the new commit (a plain fast-forward on top of d13f4b92).

Percent escapes in format_labels

I went with encoding. parse_labels splits on , and ; and then percent-decodes each label once, so format_labels now percent-encodes exactly those three characters in a label: % → %25, , → %2C, ; → %3B. Every label that normalize_attribution keeps now survives the round trip:

>>> format_labels({'team': 'release%41'})
'team=release%2541'
>>> parse_labels(format_labels({'team': 'release%41'}))
{'team': 'release%41'}

This also removes the earlier separator rule. A label containing , or ; used to be dropped with a warning; now it is encoded and sent (design, ops → team=design%2C ops). Parsing is unchanged, so headers that clients already send mean what they meant before.

Tests in tests/test_attribution.py:

  • test_format_labels_roundtrip_keeps_percent_and_separators, parametrized over release%41, 100%, %%, %2541, design, ops, a;b, k=v and 50% off; ,x. Each must come back unchanged next to a second label.
  • test_format_labels_encodes_only_what_the_parser_reads pins the exact header text.

On the previous head, 6 of these cases fail (release%41, %2541, 50% off; ,x, design, ops, a;b and the exact-output test). All of them pass with the fix.

Test evidence

Run in a sealed container (Python 3.13, no network, sources copied in, nothing mounted) on the tree of the new commit:

  • Every test file that mentions attribution (40 files, including test_attribution.py, test_attribution_savings.py, test_extension_attribution.py, test_pipeline_outcome_stage.py, test_proxy_savings_history.py and test_prometheus_label_escaping.py): 963 passed, 2 skipped.
  • ruff check and ruff format --check on both changed files are clean.

The three flagged check runs

Run 131027444949 (template), run 131027444646 (label) and run 43172051543 (PR Governance) on d13f4b92 were cancelled, not failed. Each was cancelled and then passed on a rerun of the same head, which is listed in the checks. The new push runs them again.

@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: needs rebase Pull request branch is behind the base branch on files it also changes status: ready for review Pull request body is complete and the author marked it ready for human review labels Nov 13, 2026
@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: ready for review Pull request body is complete and the author marked it ready for human review labels Nov 14, 2026
@github-actions github-actions Bot added status: ready for review Pull request body is complete and the author marked it ready for human review and removed status: ready for review Pull request body is complete and the author marked it ready for human review labels Nov 14, 2026

@JerrettDavis JerrettDavis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The percent-escape fix handles the earlier examples, but encoding can now exceed the parser's header limit and silently change attribution. Reproduce with labels = {f"d{i}": "%" * 128 for i in range(16)} followed by parse_labels(format_labels(labels)). All 16 labels survive normalization; expected the same mapping back. Actual: the formatter emits 6,228 characters, the parser reads 4,096, only 11 labels remain, and d10 shrinks from 128 characters to 68. Please reconcile the encoded-size and parser limits, or explicitly reject oversized formatter output, and add a boundary regression. The normal-package regression fails on this head. Existing attribution tests passed across two runs: 69 normally and 12 proxy tests in supported Python-only mode; native compression and live provider traffic were not tested.

@JerrettDavis JerrettDavis added area: cli Touches cli files; triage routing, not a CODEOWNERS assignment. area: compression Touches compression files; triage routing, not a CODEOWNERS assignment. area: pricing Touches pricing files; triage routing, not a CODEOWNERS assignment. area: proxy Touches proxy files; triage routing, not a CODEOWNERS assignment. enhancement New feature or request risk: high Scope estimate: security, dependencies, release, broad or critical runtime change. size: large More than 500 added/deleted lines. status: code changes requested labels Nov 14, 2026
@github-actions github-actions Bot added the status: ready for review Pull request body is complete and the author marked it ready for human review label Nov 14, 2026
@mosatch

mosatch commented Nov 14, 2026

Copy link
Copy Markdown
Contributor Author

@JerrettDavis thanks, good catch. Fixed, together with a few more things we found reviewing the rest of the PR ourselves. Everything is a fast-forward: a merge of upstream/main (it had started to conflict), then one commit per fix.

The header limit

LABELS_HEADER_MAX_LENGTH is now computed from the other limits instead of being a fixed 4096:

LABELS_HEADER_MAX_LENGTH = MAX_DIMENSIONS * (DIMENSION_NAME_MAX_LENGTH + len("=") + 3 * LABEL_MAX_LENGTH) + (MAX_DIMENSIONS - 1) * len("; ")
# 16 * (32 + 1 + 384) + 15 * 2 = 6702

That is the longest header format_labels can build from ASCII labels: every name and label at full length, with every character percent-encoded. So the parser reads anything the formatter builds in full, and the limit follows if the other limits ever change.

The parser also stops shortening labels. When a header is longer than the limit, it is cut back to the last separator, so a pair cut by the limit is dropped instead of being booked under a shortened label (your d10 case). That applies to headers clients build by hand too.

format_labels now also percent-encodes non-ASCII, so the header is plain ASCII. A non-ASCII character can encode to up to 12 characters, so for those labels the formatter raises ValueError instead of building a header the parser would cut.

Tests (tests/test_attribution.py):

  • test_format_labels_reported_overflow_case_roundtrips: your exact reproduction.
  • test_format_labels_longest_output_is_read_whole: the longest possible header is exactly the limit and comes back unchanged, through parse_labels and resolve_attribution.
  • test_a_pair_cut_by_the_header_limit_is_dropped_not_shortened: the boundary on both sides, including a separator exactly at the limit.
  • test_format_labels_encodes_non_ascii (café, 東京, emoji, mixed) and test_format_labels_refuses_a_header_the_parser_would_cut.

Other fixes from our own review

  1. Deferred gateway outcomes were booked under another request's labels. With relay_usage, the outcome is recorded later: by the response half, or by whichever request's register()/get() sweeps the expired turn. Both run in a different request's context, so that request's labels and conversation got the savings. A PendingTurn now keeps a copy of its request half's context and the draft is recorded in it. An orphaned draft is recorded with no labels. Tests: test_an_expired_draft_is_booked_under_its_own_requests_labels, test_an_orphaned_draft_is_booked_with_no_labels and, end to end through /v1/compress/response, test_deferred_outcome_is_booked_under_the_request_halfs_labels.
  2. Per-label token totals left out deferred tool schemas. by_dimension counted their dollars but not their tokens, so a label could show $0.06 next to 0 tokens, disagree with headroom savings --by, and be the first one evicted. It now has tool_tokens_saved and headline_tokens_saved, as by_model does, and ranks and evicts by the sum. The dashboard card uses the sum. Test: test_a_label_counts_deferred_tool_schemas_like_the_ledger.
  3. Labels were trimmed before they were cut to 128 characters, so a space at position 128 survived. Cleaning the result again gave a different label, which could split one label into two counters across a restart. They are now cut first and trimmed second. Test: test_a_label_cut_to_length_keeps_no_trailing_space.
  4. The conversation id was percent-decoded twice (abc%2541 became abcA). set_current_conversation now takes the already-decoded value. Covered in test_snapshot_falls_back_to_the_client_named_conversation.
  5. The labels switch used up one of the 16 dimension slots, so as the 17th entry it was dropped without notice. It no longer counts against the limit. Test: test_the_labels_switch_does_not_take_a_dimension_slot.
  6. Dimension names starting with - or . are refused. A client could create -project through labels, but an operator could never list it, because -x means "remove x". Tests: test_sanitize_dimension_name and test_a_name_starting_with_a_dash_cannot_be_sent_as_a_label.
  7. unknown is not accepted as a label. The ledger already shows it for "no label", so --where host=unknown mixed the two together. Test: test_unknown_is_not_a_label.
  8. Removed the HEADROOM_LABELS constant. It was documented as something headroom wrap reads, but nothing read it.
  9. Docs: the header limit and the label rules above, the per-label token fields, and a correction. A replaced resolver's label wins in request-log tags, but when it finds nothing the tag still shows what the proxy detected.

Merge with upstream

upstream/main (fda75de5) conflicted in two places, both from #4859:

Test evidence

Everything ran in a sealed container (Python 3.13, no network, sources copied in, nothing mounted), on the tree of the new head:

  • Broad run: every test file that mentions attribution, per_project or /stats-lifetime, plus tests/gateway/ and test_gateway_turn_unit.py (60 files): 1753 passed, 6 skipped, 1 failed.
  • The one failure is tests/gateway/test_compress_turn_seam.py::test_contract_disabled_by_env (502 where it expects 404/405). It fails the same way on the previous head 9c4f2f1f in the same container, and it does not touch attribution.
  • Before the fix: with the previous head's code and the new tests, 20 of the new tests fail.
  • Each commit: built one at a time on the previous head, each commit passes test_attribution.py, test_attribution_savings.py, test_gateway_turn_unit.py and gateway/test_contract_response_half.py.
  • Lint: ruff check and ruff format --check are clean on every changed file.

@github-actions github-actions Bot removed the status: ready for review Pull request body is complete and the author marked it ready for human review label Nov 14, 2026

This branch has not been deployed

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

Labels

area: cli Touches cli files; triage routing, not a CODEOWNERS assignment. area: compression Touches compression files; triage routing, not a CODEOWNERS assignment. area: pricing Touches pricing files; triage routing, not a CODEOWNERS assignment. area: proxy Touches proxy files; triage routing, not a CODEOWNERS assignment. enhancement New feature or request risk: high Scope estimate: security, dependencies, release, broad or critical runtime change. size: large More than 500 added/deleted lines. status: code changes requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants