mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-09-17 23:52:36 +00:00
18cbeb907c
* feat(eval): record provider-native usage at the gateway, not after translation
The benchmark reads token counts out of Claude Code's session output, which is
Anthropic-shaped whatever actually served the request. That holds until the
upstream is OpenAI, because the two providers do not merely name their fields
differently - they mean opposite things by them:
Anthropic: total_input = input_tokens + cache_creation + cache_read
(input_tokens is the UNCACHED remainder; cache fields ADD)
OpenAI: total_input = input_tokens
ordinary = input_tokens - cached - cache_write
(input_tokens is the WHOLE; cache fields are SUBSETS)
Adding OpenAI's three double-counts; subtracting Anthropic's under-counts. One
shared struct cannot be right for both, so the seam goes at the gateway, on the
far side of the translation: a LiteLLM callback appends each upstream request's
usage verbatim, along with the model that actually answered, the response id and
the cell it belongs to. Normalization is derived offline from that record, so the
derivation can be revisited without re-running a paid sweep.
Two rules the tests encode literally.
The native object is authoritative. The callback stores it unflattened,
unrenamed and unsummed. Reasoning tokens are kept as the decomposition of output
tokens they are, not added to them a second time.
A field nobody reported is unknown, never zero. A stored cache_read of 0 used to
mean either "the provider said zero" or "our adapter never looked" - the first
says caching is not working, the second says we cannot tell. NormalizedUsage
therefore uses None, and refuses to compute the ordinary portion when a term is
missing rather than subtracting an invented zero.
Mutation-checked three ways. Giving OpenAI Anthropic's arithmetic fails four
tests. Making unknown fall back to zero fails the unknown test. Dropping
input_tokens_details in the callback fails the end-to-end accounting test with
"assert None == 3000" - it goes unknown rather than passing with zeros, which
was the point of the exercise.
The actual model is recorded separately from the requested role because several
Claude role names map onto one upstream model here; pricing must follow what
answered. Cost is deliberately NOT stored: prices change, and tokens plus a
versioned pricing table can answer both what a past run cost and what the same
usage would cost today, without rewriting historical evidence.
The callback never raises. A cell that fails still spent money upstream, and
losing the accounting because a log write failed is the worse outcome. Failed
requests are recorded too.
No caching configuration, model, skill or promotion change: this installs the
thermometer without altering the experiment. 538 eval tests pass plus 27 gateway
tests; ruff clean. The two test_model_gateway.py failures are environmental -
litellm[proxy]'s console script is absent in this venv - and predate this branch.
* fix(eval): drop the accidentally committed .venv symlink
I symlinked eval/.venv at a sibling worktree's virtualenv to avoid rebuilding
it, and git add -A committed the symlink. .gitignore lists ".venv/" with a
trailing slash, which matches a directory and not a symlink, so nothing stopped
it.
That broke eval / containment (windows), where uv then refused to create the
environment: "failed to create directory eval\\.venv: Cannot create a file when
that file already exists". A machine-specific absolute path had no business in
the tree in the first place.
Removed, and .gitignore now also lists the bare name so the same slip cannot
repeat.
* Address PR review feedback (#3220)
Forward the usage environment into the proxy. This is the one that mattered:
the callback returns immediately when GITNEXUS_BENCH_PROVIDER_USAGE is absent,
the proxy runs as its own process, and Popen(env=...) REPLACES the parent
environment rather than extending it. The gateway's allowlist carried the
OpenAI and master keys and nothing else, so the callback loaded, found no
destination, and silently recorded nothing on every request. The accounting
looked configured and measured nothing at all.
My tests could not see it. They set the variable in-process and called the
logger directly, so none of them ever crossed the subprocess boundary the
feature actually runs behind. The new test drives OpenAIGateway.__enter__ with
Popen captured and asserts each variable reaches the child - and that the
result is still an allowlist rather than the inherited parent environment,
since forwarding by name is what keeps the credential boundary explicit.
Resolve the provider label into an adapter key. The callback recorded
LiteLLM's custom_llm_provider, which is "openai", while the adapter table is
keyed "openai-responses" - so nothing the logger wrote could have been
normalized. The end-to-end test hid this by passing OPENAI_RESPONSES by hand
instead of using the provider the log recorded; it now uses the logged value,
which is what makes the mismatch visible.
The label alone cannot pick an adapter: LiteLLM reports "openai" for Chat
Completions as well, and the two report usage differently. canonical_provider
combines the label with the call type and returns None when it cannot resolve
one, so normalize_usage refuses rather than guessing token semantics. Both are
stored - provider_label is what LiteLLM said, provider is the adapter key.
The shared env-var names moved into provider_usage.py so model_gateway can
import them without importing litellm, which only the in-proxy callback needs.
Mutation-checked. Removing the forwarding loop fails the gateway test; using
the raw label as the adapter key fails two.
656 eval tests pass, ruff clean. The two test_model_gateway.py failures are the
environmental ones - litellm[proxy]'s console script is absent here, which is
also why the new test patches the argv builder to reach Popen at all.
* fix(eval): stop recording a cell id the proxy cannot know
Setting out to build the correlation this PR was missing - cell usage as the
sum of its upstream requests - turned up that the field it would have been
built on cannot hold what its name claims.
attach_openai_gateway wraps the whole sweep (runner.py:2122), so ONE proxy
serves every cell, and its environment is fixed for that process's lifetime.
Cells run concurrently under --workers and interleave requests through it. A
cell id forwarded at launch is therefore the same constant on every event the
callback ever writes - not an attribution, just a label that looks like one.
Worse than absent, because a reader would trust it.
So GITNEXUS_BENCH_CELL_ID is gone rather than left to be wired up later. What
remains is honest about its scope: sweep_id is genuinely sweep-wide, and
session_id is the per-request half - the only thing that can attribute a
request to a cell, since anything read from the environment is shared by all of
them. It is recorded even when the provider supplies nothing, because knowing
attribution is unavailable is itself a fact about the run.
Pinned by a test asserting the forwarded set contains no per-cell variable, so
a later change does not reintroduce one and quietly stamp a single value across
concurrent cells.
What this leaves open, stated plainly: per-cell attribution is NOT built, and
cannot be until a per-request identifier is available. Whether Claude Code
propagates a session identifier through the proxy is unverified - determining
it needs a real session against the gateway, which is a paid run. Sweep-level
totals and per-request cache ratios do not need it, and those are what the
caching question actually turns on.
658 eval tests pass, ruff clean; the two test_model_gateway.py failures remain
environmental.
* fix(eval): keep the usage callback importable the way LiteLLM loads it
CI caught a regression I introduced: "ImportError: Could not import handler
from provider_usage_callback", and the proxy exited before becoming ready.
Moving the shared constants into provider_usage.py, I imported them from the
callback with "from .provider_usage import ...". But LiteLLM resolves a dotted
callback through spec_from_file_location against the config directory, so the
copied file runs as a top-level module with no parent package and no sys.path
entry - the relative import raises and the gateway never starts. The module's
own docstring says it is deliberately self-contained for exactly this reason,
and I broke that invariant while tidying.
The in-package tests could not see it. They import
workflow_bench.litellm_usage_callback, where the relative import resolves
fine; the failure only exists on the path where the file is copied and loaded
standalone.
The callback carries its own literals again. Two tests keep that honest: one
loads the copied file the way LiteLLM does - by path, as a top-level module -
so an import that only works in-package fails there, and one asserts the
copied constants and the provider resolver still agree with the canonical
copies in provider_usage.py, so the deliberate duplication cannot drift
silently.
Mutation-checked: restoring the relative import reproduces CI's exact error.
660 eval tests pass locally; the two remaining test_model_gateway.py failures
are the environmental ones (litellm[proxy]'s console script is absent here,
which is also why this never reproduced locally).
* test(eval): import the installed callback instead of grepping it
Two review findings on the same weakness, both correct.
The install test asserted "class ProviderUsageLogger" appeared in the copied
file's text. That passes whenever the string is present, including when the
module cannot load at all - which is precisely how a package-relative import
got through review here and took the proxy down. It now loads the copy the way
LiteLLM does, by path as a top-level module, and checks the handler instance
the config actually names.
The gateway-forwarding test built its work directory with tempfile.mkdtemp(),
which nothing removed, so every run left the generated config and the copied
callback behind in the system temp directory. It uses the pytest-managed
tmp_path fixture like its neighbours.
660 eval tests pass; the two test_model_gateway.py failures are the
environmental ones.
* fix(eval): record failures on the synchronous callback path too
ProviderUsageLogger overrode both async hooks and the sync SUCCESS hook, but
not the sync failure hook. On that path failures fell through to CustomLogger's
base implementation and were never appended - so a sweep recorded its
successes and quietly understated what it spent, since a failed request is
billed all the same. That contradicts the module's own stated reason for
handling failures at all.
The failure test could not have caught it: it called _append directly, which
exercises neither public hook. Both failure tests now drive the hooks LiteLLM
actually calls, and a new one walks all four - sync and async, success and
failure - asserting each records in order. Removing the sync failure hook fails
both.
661 eval tests pass; the two test_model_gateway.py failures remain
environmental.
---------
Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
|
||
|---|---|---|
| .. | ||
| __init__.py | ||
| bench_fixtures.py | ||
| conftest.py | ||
| test_ce_plugin_runtime.py | ||
| test_comparator_reuse.py | ||
| test_errors.py | ||
| test_evolve.py | ||
| test_mcp_bridge.py | ||
| test_measure_evolution_cost.py | ||
| test_model_gateway.py | ||
| test_oracle_assets.py | ||
| test_parse_run_id.py | ||
| test_process_control.py | ||
| test_promotion_apply.py | ||
| test_property_based.py | ||
| test_proposer_sandbox.py | ||
| test_provider_usage.py | ||
| test_provider_usage_capture.py | ||
| test_reuse_round_trip.py | ||
| test_review_corpus.py | ||
| test_review_scoring.py | ||
| test_runner_hardening.py | ||
| test_sanitized_graph.py | ||
| test_session_progress.py | ||
| test_sweep_finalization.py | ||
| test_task_assets.py | ||
| test_tool_scripts.py | ||
| test_workflow_bench.py | ||
| test_workflow_bench_evolution.py | ||
| test_workflow_bench_sessions.py | ||