mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-10-02 02:11:29 +00:00
* feat(eval): a scriptable stand-in for Anthropic and OpenAI Every defect this harness shipped last round was invisible to its own tests for one reason: the tests exercised a layer BELOW where the code runs. The usage log was never written because the proxy is a subprocess with a constructed environment. The callback could not be imported because LiteLLM loads it by path, not as a package. Failures went unrecorded because only the async hook was overridden. CI or review caught all three; no unit test could, because each called the function directly instead of driving the path that calls it. This closes that gap without spending money. It speaks the two wire protocols the harness actually depends on - Anthropic Messages, streaming and not, and OpenAI Responses - so a run can go through the real sandbox, the real CLI, the real gateway and the real usage callback with only the model faked. The runner already supports pointing at it: --base-url is the same path the free-model proxy documentation uses. Scripted rather than simulated. A test decides what the model says, which tools it asks for, and exactly what usage it reports. That last part is what makes provider-native accounting testable at all: real cache hits are not reproducible on demand, but a declared cache_read of 44,000 is. One Reply served down both protocols is also the cleanest demonstration that the same billed work is stated as a sum on one side and as a whole on the other. Tool blocks are the mechanism for artifact-producing cells. The CLI runs what it is asked to run, so a scripted Write block makes it write that file inside the sandbox for real - no model deciding anything. The end-to-end test drives the real proxy against the mock and asserts the usage log records the provider's own arithmetic through the Anthropic-shaped translation. It SKIPS here, because litellm's console script is absent in this environment, so it is unverified until CI runs it - the same footing the bubblewrap canary started on, and that one found a real bug on its first CI run. Not yet built: driving a whole sweep against this. That needs a scripted reply sequence that carries a cell to a scored artifact, which is the next step and the point of the exercise. 668 eval tests pass, 17 skipped; the two test_model_gateway.py failures are the pre-existing environmental ones. * test(eval): run a real session against the scripted provider The mock only proves something once the harness runs against it. This adds the stand-in CLI and the first integration tests that use it, so a session goes through the real code with only the model faked. tests/fixtures/fake_claude.py does what the CLI does at the two boundaries the harness depends on: it calls ANTHROPIC_BASE_URL for a turn, EXECUTES the tool blocks that come back, and prints the stream-json sequence the parent parses. Everything between - the session runner, the event-stream parse, the usage extraction, the artifact capture, the scorer - stays real. Four tests, chosen for the layers that have actually broken here: the usage a provider reported survives to the row, a scripted Write produces an artifact parse_review_output accepts, the prompt the harness meant to send is what arrived, and an upstream 529 lands as a failed session rather than a usable measurement. Writing the stand-in found two things worth keeping. The prompt arrives on STDIN under "-p --input-format text"; scanning argv for a non-flag token picks up a flag's value instead, and the prompt-fidelity test is what caught it. And three of these tests had been holding a sandbox they never applied, since no command_prefix is passed - that implied coverage which was not there, so the sandbox is gone from them and stays only in the artifact test, which needs its review directory. What these do NOT cover, checked rather than assumed: making the stand-in write in place instead of atomically still passes. On the host-unsafe backend there is no read-only mount to refuse it, so the atomic-write requirement remains a bubblewrap mount property that only the real-sandbox canary can prove. Dropping cache_read from the recorded usage does fail, so that half is genuinely pinned. 672 eval tests pass, 17 skipped; the two test_model_gateway.py failures are the environmental ones. * fix(eval): the usage adapter read a shape the callback never receives Running the gateway against the scripted provider proved the accounting merged in #3220 does not work, and the same run showed why nothing had caught it. LiteLLM does not hand a logger the upstream body. It normalises usage into its own Chat-Completions-shaped object first, so an OpenAI Responses reply reaches the callback as prompt_tokens / prompt_tokens_details.cached_tokens - never the input_tokens / input_tokens_details the shipped adapter reads. Every field came back unknown. The observed call_type is "anthropic_messages" as well, because Claude Code calls the Anthropic-shaped endpoint, so canonical_provider returned None and normalize_usage would have refused outright. Both were assumptions about a boundary I had only read about. The unit tests agreed with them because their fixture was written in the same wrong shape, so producer and consumer were consistent and both wrong - the exact failure the producer/consumer round trip exists to catch, one layer further out. Adds a LITELLM_NORMALIZED adapter for the object that actually arrives. The arithmetic is still OpenAI's - prompt_tokens is the whole, the details are subsets - so ordinary input is recovered by subtraction. The Responses adapter stays for a raw upstream body, which the mock still serves and tests directly. An unrecognised provider is still refused rather than guessed. The fixtures now carry the measured shape, and the end-to-end test asserts it through a real proxy: 48k prompt tokens with 44k cached is read back as 3k ordinary rather than as silence. 676 eval tests pass, 16 skipped, none failing. * test(eval): run a whole sweep offline, with negative controls The layers between a model turn and a promotion decision had never been exercised together. Unit tests covered each alone, and the paid runs that would have covered the composition kept dying, so the contracts BETWEEN them went unverified - which is where this harness has repeatedly shipped bugs. Drives runner.main() the way the workflow does. Real task selection, hidden oracle capture, sandbox, CLI subprocess, artifact capture, scoring against the oracle, aggregation, health guard and promotion gate. Only the model is scripted. Getting to green meant satisfying nine real contracts nothing had exercised end to end, and each failure was the harness correctly refusing bad evidence: --unsafe-no-bwrap is restricted to the paired review arms; ce_* needs a plugin carrying ce-plan, ce-work and ce-code-review; candidate_* needs an overlay; the clone needs .gitnexus/meta.json with indexedAt and lastCommit; the evidence gate needs a Skill request with a non-error result; review findings need exactly ten fields with severity in critical/high/medium/low; and the hidden labels use a DIFFERENT schema from the review output - line_start/line_end, six fields. That last one only a real run surfaces. Three negative controls, because a scorer that cannot be wrong measures nothing. A finding in the wrong place is tp=0 fp=1 fn=1 and oracle-failed, while its evidence stays VALID - being wrong is a quality result, not a broken measurement. Approving defective code is a miss with no false positive, and precision is None rather than 0, because it is undefined with no predictions. One run cannot promote: the gate says it needs three valid paired runs. A fourth control exists because a mutation demanded it. Forcing skill_was_invoked_events to return True left every other test here passing, so nothing pinned the gate that separates measuring a SKILL from measuring a model. Writing it turned up behaviour worth recording rather than assuming: a skill-not-invoked row still carries its score AND still counts toward the arm median, because aggregate() drops EXCLUDED_ERROR_KINDS and evidence_valid=False and skill-not-invoked is neither. The health guard stops the sweep, so a single-run sweep cannot promote on it, but a mixed run's median would include a cell whose skill never ran. Pinned as-is so it cannot change silently in either direction; changing it is a promotion-semantics decision, not a test fix. Two provisioning steps are stubbed and neither is harness logic: the pinned runtime mounts (no node_modules in a worktree) and the sanitized graph build (needs the gitnexus CLI at a mounted path). Containment is host-unsafe here; bubblewrap stays with the real-sandbox canary. 681 eval tests pass, 16 skipped, none failing. Runs in ~18s. * fix(eval): an uninvoked skill must not move the arm's quality median Found by the offline sweep: a skill-not-invoked row still carried its score into the arm's quality median. aggregate()'s filter dropped EXCLUDED_ERROR_KINDS and evidence_valid=False, and skill-not-invoked is neither, so an arm could be credited for a review it never performed with the skill under test - which is the one thing an arm exists to measure. Excluded from the QUALITY metrics only. Cost and duration still count that row, because the session really ran and really was billed, and the promotion gate still sees it, because it has its own vocabulary for a candidate that never loaded its skill. Two wider fixes were tried and abandoned, both because the tests said so rather than because I reasoned it out first. Reusing the health guard's evidence_failed predicate also excluded transcript-missing rows, but test_aggregate_excludes_session_error_rows_from_medians pins those as counting: that session ran, only its transcript is unverifiable. Excluding the row from `valid` outright turned a candidate whose skill never loaded from keep_incumbent into insufficient_evidence - the safety property held either way, but the decision vocabulary is promotion semantics and not mine to change on a measurement fix. Mutation-checked: putting the rows back into the quality median fails the new test. Both directions asserted, since a filter that excludes everything would also pass - a wrong-but-valid review still moves quality, because being wrong is exactly what a quality median should reflect. 682 eval tests pass, 16 skipped. * test(eval): run the offline sweep unstubbed in the job that can, and probe CLI identity Items 5 and 6 turned out to be one change. The containment (ubuntu) job already installs bubblewrap, the pinned Claude CLI, node_modules and a built GitNexus - everything the sweep's two provisioning stubs stand in for. So the stubs are not a property of the test, only of a machine that lacks those things. GITNEXUS_REQUIRE_FULL_SWEEP=1 makes the sweep run with nothing stubbed: real containment instead of --unsafe-no-bwrap, the real runtime mounts, the real sanitized graph. Set in that job, following the GITNEXUS_REQUIRE_BWRAP_CANARY pattern already there. The gate FAILS on a missing piece rather than degrading to the stubbed path, which is the point - a green tick that silently tested less is what the bubblewrap canary was written to prevent. Verified both states here: default green, and gate-on fails on this machine rather than skipping, since it cannot create user namespaces. Item 7 is an experiment, not an answer. Per-cell attribution needs an identifier that travels WITH the request, because one proxy serves the whole sweep and anything read from its environment is identical for every call. What the real CLI sends is not documented anywhere I can check, and guessing a wire format is exactly how the last three accounting bugs happened. So the probe drives the REAL pinned CLI against the mock and records the identity-bearing headers and body keys that arrive. It asserts only that a request was made; the recorded evidence is the deliverable, and the job log preserves it. Skips without CLAUDE_CANARY_BIN. Two guards caught this rather than review: the repo pins the containment job's env and its exact test list, so both had to be updated deliberately - which is the guard working, not friction. 682 eval tests pass, 17 skipped. * test(eval): make the offline sweep cross-task, so a scheduler change is checkable The sweep fixture had one task, and a single task cannot show the thing a cross-task scheduler changes: waves are per-task, so ordering, packing and a breaker spanning a task boundary are all invisible with one. A second task with its defect in a DIFFERENT file, and its own hidden labels, makes per-task routing observable. The scripted reply is now task-aware, which matters for the same reason: replying with the first task's finding scores the second task wrong. The load-bearing assertion is that each task scored against ITS OWN oracle. That is the dangerous failure mode of interleaving cells from different tasks - a mis-routed context or artifact scores one task against another's labels, and every row still looks green. Mutation-checked: pointing every cell at the first task's oracle snapshot fails it. This is the safety net the packed-scheduler wiring needs. Measured earlier against the real sweep_packed_cells, that change is worth -27% on a cold sweep and -37% weekly, with breaker fidelity holding at three injected failure positions - but it restructures a 125-line loop across ~92 names that also holds graph prefetch, reuse selection, oracle staging and the canary drop. Landing that on top of a one-task fixture would have been unverifiable, which is why this comes first and separately. 682 eval tests pass, 17 skipped. * fix(eval): commit the stand-in CLI's executable bit The file was created and chmod +x'd locally, but committed 100644 - so the mode existed only in my working tree. Any fresh checkout, CI included, gets a non-executable file and every cell dies with "required executable is not an executable regular file". Found by accident: checking out origin/main and back to compare a flaky test restored the file from the index and stripped the bit, which turned 5 green tests into 9 failures. Without that detour this would have failed on the first CI run instead. Same shape as the bugs this branch exists to catch - something that works only because of local state, breaking where the code actually runs. * fix(eval): apply code review findings Seven local reviewers and an independent cross-model pass. The headline is that a fix I added in this branch was worse than the gap it closed. Reverted the aggregate() quality-median filter. Excluding skill-not-invoked rows from the quality metrics left valid_runs and excluded_runs still counting them, so the promotion gate saw N clean runs while the median came from fewer. The dropped rows are systematically an arm's worst, so it biased toward PROMOTING - reproduced: one real run at 0.9 plus two uninvoked rows at 0.0 gave the gate 3 valid runs, zero exclusions and a 0.9 median, flipping keep_incumbent to promote. Three verdict fields compounded it: they are all() reducers still reading the wider set, so one uninvoked cell flipped a whole arm. Five reviewers found the two halves independently. Closing it honestly needs a scored-run count plus a paired-equality check in the gate, which is promotion semantics rather than an aggregation fix. The gap is now pinned by a test that states why the half-fix was reverted. Stopped forging the absence of CI. The runner refuses --unsafe-no-bwrap when CI is set because that mode runs sessions with bypassPermissions behind a boundary its own docstring calls "not a security boundary"; the sweep test deleted CI to get past it, so eval / locked pytest ran an uncontained agent sweep on the runner holding the checkout and credentials. It skips under CI instead - the containment job still runs it for real with GITNEXUS_REQUIRE_FULL_SWEEP=1. The stand-in CLI was lying in three ways. It never set is_error, so a refused write read as a completed one. It had no Skill branch at all, so honoring is_error revealed the evidence gate had been satisfied by a tool the fixture never ran - the gate was measuring the fixture, not a skill. And a reply with no usage became four zero-valued fields plus a fabricated cost, which is exactly the unknown-is-not-zero confusion the accounting it feeds exists to prevent. A provider failure also crashed the subprocess with no terminal result event. The identity probe never ran anywhere. test_mock_provider.py was in no job's file list, and the only job setting CLAUDE_CANARY_BIN runs a fixed list. My commit message claimed the next containment run would produce the answer; it would not have. Now wired in, with the CI-shape test updated to pin it. Also: the regex-miss fallback wrote a predictable name in shared /tmp through a symlink-following stage, now scoped to the test's own directory; and the canonical_provider docstring plus the callback comment still asserted a call_type branch the code no longer has. Deferred as design decisions rather than review fixes: the containment sweep uses the stand-in CLI rather than the pinned real one, the full-sweep path bypasses the gateway so native usage accounting is unexercised there, _normalize_litellm duplicates the Responses algorithm, and OPENAI_RESPONSES is now unreachable from canonical_provider. 682 eval tests pass, 17 skipped, ruff clean. * fix(eval): carry scripted tools over the Responses protocol Review round on #3235. Three real items; five more were already fixed ina20f94e1cand are answered on their threads rather than re-fixed. `_openai_response` emitted only an `output_text` item and never read `reply.tools`, so a reply scripted with a Write or Skill crossed the gateway with the tool silently dropped. Responses is the protocol the gateway is configured for BECAUSE it carries tool use, so the mock was wrong about the wire on the one path that matters most. Function-call items now accompany the message. Mutation-checked: reverting the emit fails the new test on "the scripted tool must cross the Responses path". The artifact session now takes `command_prefix` and `require_pid_namespace` from the sandbox the way `run_arm` does instead of calling `run_claude` bare. On host-unsafe `command_prefix_for` returns `[]` by construction, so this pins the wiring, not the isolation - the comment says so rather than implying more. CodeQL's three unused-variable reports on one line were one finding: a call whose result is entirely discarded. Unpack nothing there. 683 passed, 17 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(eval): forward the usage the provider reported instead of zero-filling it Review round on #3235; all three findings valid. The stand-in CLI defaulted absent cache fields to 0. That fabricated a complete measurement out of an incomplete reply, and the second-order effect was worse than the first: `runner_sessions` requires all four USAGE_FIELDS before it calls a session measured, so a stand-in that always emitted four fields made that guard unfirable from any offline test. It was always satisfied. It now forwards exactly what arrived. `Reply`'s cache fields accept None to script absence, since a consumer that cannot tell "omitted" from "zero" is the bug this harness exists to catch. Mutation-checked: restoring the zero-fill fails the new test. Also corrected a comment claiming aggregate() excludes skill-not-invoked rows from the quality median. It does not - that was the filter reverted ina20f94e1cfor inverting a promotion, and the comment survived the revert describing the opposite of what the test pins. Dropped an unused monkeypatch fixture arg. 684 passed, 17 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(eval): keep an omitted cache field omitted on the Responses wire Review round on #3235. The main finding is a miss in my own previous commit: that one taught the Anthropic path to forward absence instead of zero-filling, but `_openai_response` still serialized both `input_tokens_details` keys unconditionally. Collapsing None to 0 is right for the arithmetic - an unreported field adds nothing to the total - and wrong on the wire, because `_int_or_none` reads an absent key as unknown and a present 0 as a measured zero. So a reply scripted with `cache_read_input_tokens=None` was indistinguishable from a provider-reported zero on exactly one of the two protocols. Half-applying the invariant was arguably worse than not applying it: the Anthropic test passing made the pair look covered. Mutation-checked: restoring the unconditional keys fails the new test. Also: the module docstring claimed the stand-in executes the tool blocks that come back, without noting Bash is stubbed; and dropped an unused tmp_path fixture arg. 685 passed, 17 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(eval): validate every usage field the stand-in forwards Review round on #3235. The guard checked input_tokens and output_tokens for type and sign, but the forwarding comprehension passed the cache fields through unchecked whenever present. The parent's well_formed test only asks whether the four keys are PRESENT, so a negative, boolean, or non-integer cache value rode into a `success` result and was recorded as a usable measurement. Same shape as the previous two rounds: the required half of a pair was handled and the optional half was not. A field good enough to report is good enough to check. Mutation-checked: dropping the added clause fails all three parametrized cases (negative, boolean, string). 688 passed, 17 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(eval): say which stand-in tools execute and which are modelled Review round on #3235. The previous commit's docstring fix said "Write and Skill really run" while correcting the Bash claim. Only Write really runs: Skill validates the request and returns a synthetic result. Fourth round of the same shape - the reported half of a pair gets fixed and the sibling keeps the overclaim. Both docstrings now name each of the three branches and what it actually does. 688 passed, 17 skipped. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
348 lines
16 KiB
Python
348 lines
16 KiB
Python
"""A whole sweep, offline: real runner, real sessions, scripted model.
|
|
|
|
The layers between a model turn and a promotion decision had never been
|
|
exercised together. Unit tests covered each in isolation and the paid runs that
|
|
would have covered the composition kept dying, so the contracts BETWEEN them
|
|
went unverified - and that is where this harness has repeatedly shipped bugs.
|
|
|
|
This drives runner.main() the way the workflow does. Everything is real: task
|
|
selection, hidden-oracle capture, the sandbox, the CLI subprocess, artifact
|
|
capture, review scoring against the oracle, aggregation, the health guard, and
|
|
the promotion gate. Only the model is scripted, through MockProvider.
|
|
|
|
Two provisioning steps are stubbed because this environment cannot supply them,
|
|
and neither is harness logic: the pinned gitnexus runtime mounts (no
|
|
node_modules in a worktree) and the sanitized graph build (needs the gitnexus
|
|
CLI at a mounted path). Containment is host-unsafe here; bubblewrap stays with
|
|
the real-sandbox canary in the containment job.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import re
|
|
import subprocess
|
|
import sys
|
|
from pathlib import Path
|
|
from types import SimpleNamespace
|
|
|
|
import os
|
|
import shutil
|
|
|
|
import pytest
|
|
|
|
from workflow_bench import oracle_assets, runner
|
|
from workflow_bench.mock_provider import MockProvider, Reply
|
|
|
|
FAKE_CLI = Path(__file__).parent / "fixtures" / "fake_claude.py"
|
|
ARMS = ("ce_review", "review", "candidate_review")
|
|
|
|
# When set, the sweep runs with NOTHING provisioning-stubbed: real bubblewrap
|
|
# containment, the real pinned runtime mounts, and the real sanitized graph
|
|
# build. The named CI job installs all three, so a missing one there is a
|
|
# regression rather than an unsupported machine - it FAILS instead of quietly
|
|
# degrading to the stubbed path, which is the whole point of the gate.
|
|
FULL_SWEEP_ENV = "GITNEXUS_REQUIRE_FULL_SWEEP"
|
|
FULL_SWEEP = os.environ.get(FULL_SWEEP_ENV) == "1"
|
|
|
|
# The runner refuses --unsafe-no-bwrap whenever CI is set, because that mode runs
|
|
# sessions with bypassPermissions behind a boundary its own docstring calls "not
|
|
# a security boundary". Deleting CI to get past that refusal would run an
|
|
# uncontained agent sweep on the runner holding the checkout and credentials, so
|
|
# the stubbed path is skipped under CI instead. The containment job sets
|
|
# GITNEXUS_REQUIRE_FULL_SWEEP=1 and takes the real bubblewrap path, so CI keeps
|
|
# its coverage; only the uncontained convenience run is given up.
|
|
pytestmark = pytest.mark.skipif(
|
|
not FULL_SWEEP and bool(os.environ.get("CI")),
|
|
reason="an uncontained sweep must not run in CI; the containment job runs it with GITNEXUS_REQUIRE_FULL_SWEEP=1",
|
|
)
|
|
|
|
# The review output and the hidden labels are DELIBERATELY different shapes -
|
|
# the labels carry line_start/line_end and no recommendation. Only a real run
|
|
# surfaces that; it is why these are written out rather than shared.
|
|
FINDING = {
|
|
"id": "f1", "severity": "high", "category": "correctness", "path": "src/sum.js",
|
|
"line": 1, "end_line": 1, "blocking": True, "scenario": "review-defect",
|
|
"evidence": "export const total = (a, b) => a - b;", "recommendation": "use a + b",
|
|
}
|
|
LABEL = {"id": "f1", "severity": "high", "category": "correctness",
|
|
"path": "src/sum.js", "line_start": 1, "line_end": 1}
|
|
SECOND_LABEL = {"id": "f2", "severity": "high", "category": "correctness",
|
|
"path": "src/scale.js", "line_start": 1, "line_end": 1}
|
|
SECOND_FINDING = {
|
|
"id": "f2", "severity": "high", "category": "correctness", "path": "src/scale.js",
|
|
"line": 1, "end_line": 1, "blocking": True, "scenario": "review-defect",
|
|
"evidence": "export const twice = (n) => n + 2;", "recommendation": "use n * 2",
|
|
}
|
|
|
|
|
|
def _git(repo: Path, *args: str) -> str:
|
|
return subprocess.run(["git", "-C", str(repo), *args], check=True,
|
|
capture_output=True, text=True).stdout.strip()
|
|
|
|
|
|
@pytest.fixture
|
|
def bench(tmp_path: Path):
|
|
"""A self-contained corpus: one repo, one task, one hidden label."""
|
|
|
|
repo = tmp_path / "repo"
|
|
(repo / "src").mkdir(parents=True)
|
|
(repo / "src" / "sum.js").write_text("export const total = (a, b) => a - b;\n")
|
|
(repo / "src" / "scale.js").write_text("export const twice = (n) => n + 2;\n")
|
|
_git(repo, "init", "-q", ".")
|
|
_git(repo, "config", "user.email", "t@t")
|
|
_git(repo, "config", "user.name", "t")
|
|
_git(repo, "add", "-A")
|
|
_git(repo, "commit", "-q", "-m", "fixture")
|
|
sha = _git(repo, "rev-parse", "HEAD")
|
|
|
|
oracles = tmp_path / "oracles"
|
|
oracles.mkdir()
|
|
(oracles / "review-fixture-defect.labels.json").write_text(
|
|
json.dumps({"schema_version": 1, "findings": [LABEL]})
|
|
)
|
|
(oracles / "review-fixture-second.labels.json").write_text(
|
|
json.dumps({"schema_version": 1, "findings": [SECOND_LABEL]})
|
|
)
|
|
|
|
tasks = tmp_path / "tasks.yaml"
|
|
tasks.write_text(
|
|
"tasks:\n"
|
|
" - id: review-fixture-defect\n"
|
|
" class: review-defect\n"
|
|
f" repo: {repo}\n"
|
|
f" ref: {sha}\n"
|
|
" prompt: Review this change and report actionable defects.\n"
|
|
' verify: test -s "$GITNEXUS_BENCH_REVIEW_OUTPUT"\n'
|
|
" oracle:\n"
|
|
' command: test -s "$GITNEXUS_BENCH_REVIEW_OUTPUT"\n'
|
|
" files: [{ source: review-fixture-defect.labels.json, target: review-labels.json }]\n"
|
|
# A SECOND task, because the thing a cross-task scheduler changes is
|
|
# invisible with one: waves are per-task, so a single task cannot show
|
|
# ordering, packing, or a breaker that spans a task boundary.
|
|
" - id: review-fixture-second\n"
|
|
" class: review-defect\n"
|
|
f" repo: {repo}\n"
|
|
f" ref: {sha}\n"
|
|
" prompt: Review the scaling helper and report actionable defects.\n"
|
|
' verify: test -s "$GITNEXUS_BENCH_REVIEW_OUTPUT"\n'
|
|
" oracle:\n"
|
|
' command: test -s "$GITNEXUS_BENCH_REVIEW_OUTPUT"\n'
|
|
" files: [{ source: review-fixture-second.labels.json, target: review-labels.json }]\n"
|
|
)
|
|
|
|
plugin = tmp_path / "ce-plugin"
|
|
(plugin / ".claude-plugin").mkdir(parents=True)
|
|
(plugin / ".claude-plugin" / "plugin.json").write_text(
|
|
json.dumps({"name": "compound-engineering", "version": "0.0.0-fixture"})
|
|
)
|
|
for skill in ("ce-plan", "ce-work", "ce-code-review"):
|
|
directory = plugin / "skills" / skill
|
|
directory.mkdir(parents=True)
|
|
(directory / "SKILL.md").write_text(f"---\nname: {skill}\ndescription: fixture\n---\nFixture.\n")
|
|
|
|
overlay = tmp_path / "overlay" / ".claude" / "skills" / "gitnexus-review"
|
|
overlay.mkdir(parents=True)
|
|
(overlay / "SKILL.md").write_text("---\nname: gitnexus-review\ndescription: fixture\n---\nCandidate.\n")
|
|
|
|
return SimpleNamespace(tasks=tasks, oracles=oracles, plugin=plugin,
|
|
overlay=tmp_path / "overlay", out=tmp_path / "out")
|
|
|
|
|
|
def _stub_provisioning(monkeypatch: pytest.MonkeyPatch) -> None:
|
|
"""Replace what this machine cannot supply - and nothing else.
|
|
|
|
Under FULL_SWEEP nothing is replaced: the runtime mounts and the graph are
|
|
built for real, so the sweep exercises containment and provisioning too.
|
|
"""
|
|
|
|
if FULL_SWEEP:
|
|
if shutil.which("bwrap") is None:
|
|
pytest.fail(f"{FULL_SWEEP_ENV}=1 but bubblewrap is absent")
|
|
return
|
|
|
|
monkeypatch.setattr(runner, "trusted_gitnexus_runtime_mounts", lambda: ())
|
|
|
|
def materialize(worktree, *, sanitized_head=None, **_kwargs):
|
|
# The one-clone registry guard reads this before any session runs.
|
|
meta = Path(worktree) / ".gitnexus"
|
|
meta.mkdir(parents=True, exist_ok=True)
|
|
(meta / "meta.json").write_text(
|
|
json.dumps({"indexedAt": "2026-09-08T00:00:00Z", "lastCommit": sanitized_head or "0" * 40})
|
|
)
|
|
|
|
def fake_graph(**kwargs):
|
|
kwargs["env"].graph_snapshots[kwargs["graph_key"]] = SimpleNamespace(
|
|
digest="fixture-graph", manifest_digest="fixture-graph-manifest",
|
|
dependency_content_digest=None, dependency_manifest_digest=None,
|
|
materialize=materialize,
|
|
)
|
|
|
|
monkeypatch.setattr(runner, "ensure_task_graph", fake_graph)
|
|
|
|
|
|
def _sweep(bench, monkeypatch: pytest.MonkeyPatch, findings: list[dict], verdict: str, *, invoke_skill: bool = True):
|
|
"""Run the real CLI against a model scripted to return `findings`."""
|
|
|
|
_stub_provisioning(monkeypatch)
|
|
monkeypatch.setattr(
|
|
oracle_assets, "ORACLE_ROOT", bench.oracles, raising=False
|
|
)
|
|
monkeypatch.setattr(
|
|
runner, "capture_task_oracles",
|
|
lambda tasks, root=bench.oracles: oracle_assets.capture_task_oracles(tasks, root=root),
|
|
)
|
|
def review_for(body: str) -> str:
|
|
# Per task: the second task's defect is in another file, so replying
|
|
# with the first task's finding would score it wrong. A cross-task
|
|
# scheduler makes which task a request belongs to load-bearing.
|
|
chosen = findings
|
|
if findings and "scaling helper" in body:
|
|
chosen = [SECOND_FINDING if f is FINDING else f for f in findings]
|
|
return json.dumps({"schema_version": 1, "verdict": verdict, "findings": chosen})
|
|
|
|
class Scripted(MockProvider):
|
|
def next_reply(self) -> Reply:
|
|
body = json.dumps(self.requests[-1].body if self.requests else {})
|
|
target = re.search(r"(/[^\s\"']*review-output\.json)", body)
|
|
skill = re.search(r"\b(gitnexus-review|ce-code-review)\b", body)
|
|
return Reply(
|
|
text="reviewing",
|
|
tools=[
|
|
# The evidence gate needs a Skill request with a non-error
|
|
# result: a review that never invoked its skill measured the
|
|
# model, not the skill.
|
|
*([{"name": "Skill", "input": {"skill": skill.group(1) if skill else "gitnexus-review"}}]
|
|
if invoke_skill else []),
|
|
{"name": "Write", "input": {
|
|
"file_path": target.group(1) if target else str(bench.out / "unmatched-review-output.json"),
|
|
"content": review_for(body)}},
|
|
],
|
|
input_tokens=2_000, output_tokens=300,
|
|
cache_read_input_tokens=7_000, cache_creation_input_tokens=1_000,
|
|
)
|
|
|
|
with Scripted() as provider:
|
|
monkeypatch.setattr(sys, "argv", [
|
|
"runner", "--tasks", str(bench.tasks), "--arms", *ARMS,
|
|
"--runs", "1", "--workers", "1", "--out", str(bench.out),
|
|
"--base-url", provider.base_url, "--anthropic-api-key", "offline",
|
|
"--claude-bin", str(FAKE_CLI),
|
|
*([] if FULL_SWEEP else ["--unsafe-no-bwrap"]),
|
|
"--model", "mock-model",
|
|
"--ce-plugin-dir", str(bench.plugin), "--ce-plugin-version", "0.0.0-fixture",
|
|
"--candidate-overlay", str(bench.overlay),
|
|
])
|
|
|
|
try:
|
|
code = runner.main()
|
|
except SystemExit as exc:
|
|
code = exc.code
|
|
|
|
rows = [json.loads(line) for line in (bench.out / "results.jsonl").read_text().splitlines()]
|
|
return code, rows, provider
|
|
|
|
|
|
def _row(rows: list[dict], arm: str, task: str = "review-fixture-defect") -> dict:
|
|
return next(r for r in rows if r["arm"] == arm and r["task"] == task)
|
|
|
|
|
|
def test_a_correct_review_scores_and_the_sweep_exits_clean(bench, monkeypatch) -> None:
|
|
"""The whole path, green: every arm measured, scored, and accounted for."""
|
|
|
|
code, rows, provider = _sweep(bench, monkeypatch, [FINDING], "request_changes")
|
|
|
|
assert code in (None, 0), f"sweep did not succeed: {code}"
|
|
tasks = {"review-fixture-defect", "review-fixture-second"}
|
|
assert len(rows) == len(ARMS) * len(tasks)
|
|
assert len(provider.requests) == len(ARMS) * len(tasks), "each cell must reach the provider once"
|
|
assert {r["task"] for r in rows} == tasks, "both tasks must have run"
|
|
|
|
row = _row(rows, "review")
|
|
assert row["ok"] is True and row["resolved"] is True
|
|
assert row["skill_invoked"] is True
|
|
assert (row["review_true_positives"], row["review_false_positives"], row["review_false_negatives"]) == (1, 0, 0)
|
|
assert row["review_f1"] == 1.0
|
|
# The provider's own numbers survived the CLI, the parser and the row.
|
|
assert row["cache_read_input_tokens"] == 7_000
|
|
assert row["input_tokens"] == 2_000
|
|
|
|
# Each task scored against ITS OWN oracle. This is what a cross-task
|
|
# scheduler puts at risk: interleaving cells from different tasks means a
|
|
# mis-routed context or artifact scores one task against another's labels,
|
|
# and both would still look "green" per row.
|
|
second = _row(rows, "review", task="review-fixture-second")
|
|
assert second["resolved"] is True and second["review_f1"] == 1.0
|
|
assert second["review_artifact"] == "review-fixture-second-review-run0.review.json"
|
|
|
|
for name in ("results.jsonl", "report.md", "promotion.json"):
|
|
assert (bench.out / name).is_file(), f"{name} was not written"
|
|
assert (bench.out / "review-fixture-defect-review-run0.review.json").is_file()
|
|
|
|
|
|
def test_one_run_cannot_promote_a_candidate(bench, monkeypatch) -> None:
|
|
"""The gate refuses on insufficient paired runs, and says so."""
|
|
|
|
_sweep(bench, monkeypatch, [FINDING], "request_changes")
|
|
promotion = json.loads((bench.out / "promotion.json").read_text())
|
|
|
|
assert promotion["run_status"] == "complete"
|
|
decision = next(d for d in promotion["decisions"] if d["candidate_arm"] == "candidate_review")
|
|
assert decision["decision"] == "insufficient_evidence"
|
|
assert any("valid paired runs" in reason for reason in decision["reasons"])
|
|
|
|
|
|
def test_a_finding_in_the_wrong_place_scores_zero_but_stays_valid_evidence(bench, monkeypatch) -> None:
|
|
"""Being wrong is a quality result, not a broken measurement.
|
|
|
|
The negative control that makes the passing case mean something: same
|
|
harness, same well-formed artifact, only the answer changed.
|
|
"""
|
|
|
|
wrong = {**FINDING, "path": "src/WRONG.js", "line": 99, "end_line": 99}
|
|
_code, rows, _provider = _sweep(bench, monkeypatch, [wrong], "request_changes")
|
|
|
|
row = _row(rows, "review")
|
|
assert (row["review_true_positives"], row["review_false_positives"], row["review_false_negatives"]) == (0, 1, 1)
|
|
assert row["review_f1"] == 0.0
|
|
assert row["resolved"] is False
|
|
assert row["error_kind"] == "oracle-failed", "a wrong answer is not a session or evidence failure"
|
|
assert row["review_evidence_valid"] is True, "the artifact was well formed; only the answer was wrong"
|
|
|
|
|
|
def test_approving_defective_code_is_a_miss_with_no_false_positive(bench, monkeypatch) -> None:
|
|
"""The other half of the control: silence scores differently from a wrong guess."""
|
|
|
|
_code, rows, _provider = _sweep(bench, monkeypatch, [], "approve")
|
|
|
|
row = _row(rows, "review")
|
|
assert (row["review_true_positives"], row["review_false_positives"], row["review_false_negatives"]) == (0, 0, 1)
|
|
assert row["review_precision"] is None, "precision is undefined with no predictions, not zero"
|
|
assert row["review_verdict_correct"] is False, "approving defective code is the wrong verdict"
|
|
assert row["review_evidence_valid"] is True
|
|
|
|
|
|
def test_a_review_that_never_invoked_its_skill_is_not_a_measurement(bench, monkeypatch) -> None:
|
|
"""The gate that separates measuring a SKILL from measuring a model.
|
|
|
|
Added because a mutation exposed it: forcing skill_was_invoked_events to
|
|
return True left every other test here passing, so nothing pinned the gate.
|
|
The artifact is written and correct in this run - only the skill request is
|
|
missing - so a pass would mean the arm scored a review it never performed.
|
|
"""
|
|
|
|
code, rows, _provider = _sweep(bench, monkeypatch, [FINDING], "request_changes", invoke_skill=False)
|
|
|
|
row = _row(rows, "review")
|
|
assert row["skill_invoked"] is False
|
|
assert row["error_kind"] == "skill-not-invoked"
|
|
assert code not in (None, 0), "the sweep must not report success on unusable evidence"
|
|
|
|
# The row still carries its own score - the artifact was well formed - and
|
|
# aggregate() DOES count it in the arm's quality median (the KNOWN GAP noted
|
|
# above aggregate(); test_workflow_bench pins the resulting 0.5). Filtering
|
|
# it out of the median alone inverted a promotion, because valid_runs and
|
|
# excluded_runs kept counting it. It counts for cost either way: the session
|
|
# ran and was billed.
|
|
assert row["review_weighted_f1"] == 1.0
|
|
assert row["review_evidence_valid"] is True
|