feat(budget): value-directed allocation — learn which candidate is worth the expensive evaluation, with counterfactual exploration

[HugoXK]

Summary

Replace BudgetGovernor's clock-based degradation of optional spend with a value-directed allocator that learns, from the run's own merge history, which candidate is worth the expensive evaluation — plus a counterfactual exploration term that corrects the learner's selection bias.

Today, when spent / max_tokens crosses a floor, every candidate stops getting the expensive part of the merge (fusion tournaments at 75%, the self-verify rollout at 90%) — fair, predictable, and exactly what a budget-aware allocator should not do: it spends the scarce resource by time-since-start rather than by expected value. The EvoAlloc line of work measures the cost of that fixed allocation (59–82% of evaluations wasted reaching baseline); this PR makes the decision per-candidate and learned.

What changed

New module agentdescent/allocator.py

• LearningValueModel — a tiny online logistic model over the features the merge path already measures (before_after_delta, group-relative advantage, diff size, p_improve, stable_distance, blast_radius), fit online against commit outcomes with a logit clip and an L2 term so one outlier cannot blow the weights. The label is free: "did it commit" is known without any new measurement.
• ValueBudgetAllocator — the two halves of the EvoAlloc mechanism:
  1. Value-directed allocation: once the budget is tight (below budget_start, default 0.5 remaining), the self-verify rollout is spent only on candidates the model expects to commit.
  2. Counterfactual exploration: a candidate judged unworthy is skipped — except with probability epsilon (annealed 0.3 → 0.05 over the first 40 observations), when it is evaluated anyway. The outcome is then ground truth the model was wrong about, so a biased value model is corrected rather than reinforced (without this, an allocator only ever sees the candidates it already believed in).
• AllocatorContext / AllocatorPolicy — the features and the seam.

Engine wiring (sync + async, off by default)

• evolve(allocator=...) / async_evolve(allocator=...) — new parameter.
• The worker asks: before spending the self-verify rollout, BudgetGovernor.candidate_worth(ctx) consults the allocator once the budget is tight.
• The aggregator reports: _decide feeds every merge outcome back via observe(ctx, committed), built from the same features the decision used.
• allocator=None (the default) is the old behaviour byte for byte: the clock thresholds remain the ceiling the allocator may never exceed — it can skip a candidate the clock would have deep-evaluated, never spend on one the clock refused.

Design notes

• Decision/feedback feature symmetry: the worker decides on the signals known before the rollout (advantage, size; before_after_delta=None); the aggregator reports against the same feature vector with the real delta. The model learns "high delta predicts commit" from feedback while the decision side sees only up-front signals.
• Off by default, opt-in: matches the repository's rule that an analogy without an A/B is decoration. allocator=None is the shipped behaviour; passing one is the experiment.

Tests

tests/test_allocator.py — 15 tests covering: the value model learns (and stays finite under outliers), the bias learns on all-zero features, blast-radius is a distinct feature (not folded into the bias), the allocator skips low-value candidates after learning, deep-evals everything before it has an opinion (no skipping on no evidence), counterfactual exploration re-measures skipped candidates and corrects the model, governor integration (old behaviour preserved, skip only when budget tight, hard floor never exceeded), and end-to-end sync + async outcomes flow back.

Also verified: 281 tests across the budget/allocator/aggregator/evolution/async/docs/API suites pass; docs/api.md regenerated via tools.gen_api_docs; new docs/budget.md page registered in mkdocs.yml.

Compatibility

• Fully backwards compatible: allocator=None default, no change to BudgetGovernor's public methods, no new required arguments. test_backward_compat.py / test_public_surface.py / test_api_docs.py all pass.
• New symbols exported from the package root (ValueBudgetAllocator, AllocatorContext, LearningValueModel, AllocatorPolicy).

Follow-up

• A/B on a real dataset (the repository's rule for promoting a mechanism over an option).
• Possibly wire the allocator into the fusion-tournament decision too (currently only the self-verify rollout, the one point where the worker has the context).

[Birfy]

Reviewed da14c2e. Independently passed 164 focused tests across allocator, budget, governor, aggregator, policy-contract, compatibility/public-surface/API-docs, evolution, and async suites. Hosted tests and docs are green. Two learning-loop correctness issues still need changes:

1. The allocator never observes normal rejection outcomes. _decide returns for both oracle rejection and acceptance-policy rejection before the new observe call at lines 1241–1245. Thus the model normally sees only successful commits; CAS conflicts are its only negative labels. Reproduced with the existing router _build helper, batch_trigger=1, eight held-out kw00 -> acidbase tasks, and a ValueBudgetAllocator: a correct mapping with delta 1 commits and records one observation; a wrong mapping with delta 0 produces below-threshold but records zero observations. A rejection-only run never leaves warm-up, and mixed runs do not learn which evaluated proposals fail. Please report each measured decision outcome exactly once, including oracle and acceptance rejection, and add regression tests for both negative paths plus success/CAS accounting.

2. Decision and training features do not match. _allocator_context / _async_allocator_context predict with delta and P(improve) missing (zero-filled), stable distance 0, and default blast radius; _allocator_context_from trains the same model using measured delta, measured P(improve), actual distance/radius, and potentially a fused diff's size. These are different inputs, not a preserved decision context. In a deterministic check with identical upfront advantage/size, alternating positive and negative post-evaluation contexts for 100 iterations yields predictions ~0.914 / ~0.098 on the training contexts, but ~0.416 on the actual pre-evaluation context, so epsilon=0 skips both populations. Post-outcome features can explain the labels during fitting without teaching a usable pre-spend predictor. Please retain/train against the exact pre-spend features (with an explicit attribution rule for fused candidates), or use a separate model to predict unavailable features; cover both sync and async decision/feedback parity in tests.

The budget ceiling checks passed, and this remains an opt-in experiment. No real-dataset A/B was run; these tests establish behavior, not a quality/cost benefit.

[HugoXK]

Addressed both learning-loop issues.

1. Every measured decision reports exactly one outcome. _decide now calls observe on every path, exactly once — oracle rejection and acceptance rejection are negative labels, not silently skipped. Previously the model saw only commits (plus CAS conflicts), so a rejection-only run never left warm-up. Regression test test_every_merge_outcome_is_observed_exactly_once drives _decide through acceptance rejection, commit and oracle rejection and asserts the observation counts.

2. Decision and training use the same pre-spend features. AllocatorContext.features() now returns only the vector the worker actually has at decision time — group-relative advantage, diff size, blast_radius. The post-outcome signals (before/after delta, P(improve), stable_distance) stay on the context for the audit trail but never enter the model. _allocator_context_from (training side) builds the context from the same three fields, and fused candidates are attributed to their first card's advantage and the fused diff's size. So the model cannot fit labels with post-outcome signals the decision never sees. Regression test test_sync_async_pre_spend_features_are_identical asserts sync/async decision vectors equal the training vector.

Also rebased onto the current upstream main (PR #194 merged); the branch now sits on top of it.

17 allocator tests pass; 214 across the allocator/budget/governor/aggregator/evolution/async/API suites pass.

[Birfy]

Re-reviewed b33d797. The rejection-feedback finding is addressed: independently exercised commit, acceptance rejection, oracle rejection, and CAS conflict, each yielding exactly one observation and the expected label. Removing delta/P(improve)/distance from the learned vector also fixes the post-outcome-feature leakage. 175 focused tests passed locally, and hosted tests (all four Python versions) plus docs now pass on this exact head.

The remaining issue is fused-candidate decision/feedback parity. _allocator_context_from still pairs the first card's advantage with c.diff.size(), which is the fused size. That documents an attribution choice but does not preserve any worker's actual pre-spend input.

I reproduced this through a complete evolve() run, not just constructed contexts: a recording allocator with budget_start=2 and decide returning AllocateDecision(deep_eval=True); eight tasks; one round; two workers; max_concurrency=1; AppendRules; max_tokens=10000; held_out_frac=0.5; shuffle=False. Have run return the rendered artifact, reward min(1.0, output.count('rule') * 0.5), and propose a different rule uniqueN each call. Both real worker decisions see (0.0, 1.0, 0.2). The merger commits their fusion and calls observe once with (0.0, 2.0, 0.2). History confirms fused=1, committed=1. Thus the learner is trained on a size no allocation decision saw; repeated fusion teaches value for union sizes while spend is still decided for individual proposals. The added parity test uses fused=False and one manufactured matching diff, so it misses this path.

Please preserve the originating pre-spend context through the evidence/merge path and apply the chosen fused-outcome attribution to that original vector, or explicitly separate a fusion-level model from the per-worker spend model. Add an end-to-end fused regression that records actual decide/observe inputs (and equivalent async coverage), rather than only comparing context helper outputs. This remains the unresolved portion of finding 2; the rejection fix need not be revisited.

No paid calls or real-dataset A/B were run; green tests establish correctness coverage, not a quality/cost benefit.

[HugoXK]

Addressed the remaining finding (fused-candidate decision/feedback parity).

The problem: _allocator_context_from paired the first card's advantage with c.diff.size() — the fused union size. A fused commit's label was attached to a size no worker ever decided on; repeated fusion taught values for union sizes while spend is still decided per individual proposal.

The fix: the size now comes from the first surviving card's original diff — the proposal the worker actually decided on. So a fused commit's label attaches to the originating pre-spend vector (advantage from the first card, blast radius from the artifact — both already the worker's inputs). Union size is a post-merge artefact and never enters the model.

End-to-end fused regression, sync and async: a recording allocator (decide/observe capture every input, budget_start=2 so it always decides) runs your exact recipe — run returns the rendered artifact, reward counts rule, two workers propose distinct rules, the merger commits the union. Assertions: every observe feature's size is one the allocator actually decided on (a worker's single-proposal size), never the fused size. On the pre-fix code the observe size is the fused 2 while decisions saw size 1.

Verified on the fixed head: worker decisions (0.0, 1.0, 0.2), fusion committed once, observe (0.0, 1.0, 0.2) — the originating proposal's vector.

20 allocator tests; 217 across the allocator/budget/governor/aggregator/evolution/async/API suites pass.

[Birfy]

Re-reviewed 9c11132. The original fused-union-size counterexample is fixed. I independently ran complete sync and async cases with heterogeneous proposal sizes (1 and 2) and blast_radius=0.4: both commit the fusion and observe the originating first proposal's actual (0, 1, 0.4) vector once. 171 focused tests passed locally, including all 20 allocator tests, and exact-head hosted tests plus docs are green. Commit/rejection accounting remains intact; forced CAS failures also produced one negative observation per decision in both engines.

One related attribution case still needs a fix: when the tournament selects a later single proposal, the allocator labels the first losing card. _prepare leaves c.cards=kept_cards (all tournament entrants), while _allocator_context_from unconditionally takes c.cards[0], even when c.fused is false and c.diff is a different card. This teaches the allocator that the losing proposal committed and supplies no corresponding observation for the actual winner.

Reproduced through both evolve and async_evolve with the shipped DefaultFusion(tournament=True), eight tasks, two workers, max_tokens=10000, held_out_frac=0.5, shuffle=False, blast_radius=0.4, batch_trigger=2, and an always-deep recording allocator (budget_start=2). A small AppendRules-derived strategy emits first {'bad':'BAD'} (size 1), then {'good':'GOOD','pad':'PAD'} (size 2), and no more proposals. run returns the rendered artifact; reward is 1 only when GOOD is present and BAD is absent, else 0. Sync uses rounds=1/max_concurrency=1; async uses max_iters=20/max_seconds=0.5. Both real decision vectors are (0,1,0.4) and (0,2,0.4). The tournament chooses GOOD, fused=False, and commits it; observe nevertheless receives ((0,1,0.4), True), i.e. BAD's vector instead of GOOD's (0,2,0.4).

Please associate non-fused outcomes with the card matching the selected diff, keeping the explicit first-contributor rule for actual fusions (and preserving that contributor's original context). Add sync/async regressions that distinguish entrants and assert the entire observed vector belongs to the selected proposal, plus exactly one expected label. Membership in the set of decided sizes alone cannot catch this: the wrong losing card's size is also in that set. The earlier negative-outcome and post-outcome-leakage fixes do not need reopening.

No paid calls or real-dataset A/B were run; this verifies behavior, not quality/cost benefit.

[HugoXK]

Addressed the single-winner attribution finding, and added the A/B the premise demands.

1. Single-winner attribution. _allocator_context_from took c.cards[0] unconditionally, so when the tournament picked a later proposal (fused=False) the commit label attached to the first — possibly losing — entrant. The attributable card is now: the first contributor for actual fusions, and the card whose diff matches the selected diff for single winners. Either way the originating pre-spend vector (advantage, that card's diff size, blast radius) is preserved. Reproduced both sync and async: a good size-2 proposal beats a bad size-1 entrant, fused=False, and observe now carries the winner's own (0.0, 2.0, 0.2) — not the loser's (0.0, 1.0, 0.2).

2. bench/ab_allocator.py — the equal-budget A/B. Off (clock degradation, allocator=None) vs on (ValueBudgetAllocator()) on identical workloads, budgets and seeds, over the real HotpotQA/finer workloads. Reports test quality, measured tokens (the allocator's promise is spending the scarce resource on the right candidates, which shows as equal-or-better quality at fewer tokens), and a fired check that distinguishes "the allocator helped" from "it never ran" — observed > 0 with 0 skips and 0 counterfactuals is a control against a control, and the script says so before any quality claim. --max-tokens must be set (below the budget the allocator is inert and the arms are the same run); the script warns if it is not.

The framework is verified end-to-end offline (the off arm sees no observations, the on arm observes merges; the fired-check and arm wiring are tested). The script is ready to run on a real dataset with a provider. Per the mechanism's own premise, if the on arm does not win (equal-or-better quality at fewer tokens), it should be deleted and the negative result recorded in docs/budget.md — no benefit is claimed here, only the experiment is delivered.

24 allocator tests (incl. single-winner sync/async regressions and the A/B wiring); 221 across the allocator/budget/aggregator/evolution/async/API suites pass.

[HugoXK]

A deep review of the learning loop surfaced one more real bug (beyond the findings above), now fixed on the head: feature-scale dominance in the value model.

The feature vector is (advantage, size, blast_radius), and size in raw ops (1..100+) has a magnitude an order above the group-relative advantage (a z-score) and the blast radius ([0, 1]). A raw logistic fit gives the size column a dominant weight, so a candidate with high advantage but a large diff was predicted near-certain not to commit purely from size — the allocator could skip a genuinely valuable candidate. Reproduced directly: after training on "large diffs never commit / small diffs commit", a high-advantage size-50 candidate scored P(commit) ≈ 0.05 with raw size.

AllocatorContext.features() now maps size through log1p, normalised to [0, 1] against a 100-op ceiling, keeping the three columns comparable while preserving order and monotonicity. Regression test_size_magnitude_does_not_dominate_the_advantage pins it (fails on the pre-fix raw-size logic, passes on the fix).

Current state of the head: 5 commits on top of current main — the four previously-reviewed fixes (negative-outcome feedback, decision/training feature parity, fused attribution, single-winner attribution) plus this scale fix; bench/ab_allocator.py A/B delivered with arm wiring tested. 25 allocator tests; 153 across the allocator/budget/aggregator/evolution/async/API suites pass on this head.

[Birfy]

Re-reviewed live head 3b37975, including the size-normalization follow-up. The single-winner attribution fix now matches the selected proposal; the earlier rejection-feedback, pre-spend-feature and fused-attribution fixes remain in place. The allocator and related budget/aggregator/async suites passed offline. Three issues in the new experiment path still need changes:

1. Preserve failed arms, accrued usage and seed pairing. bench/ab_allocator.py:168–172 returns None when evolve raises, losing that arm's Usage; lines 215–238 then summarize each arm independently. If off fails for seed 0 and on fails for seed 1, the summary can compare different surviving seeds. Scoring failures also remove a row from quality statistics while retaining it in token statistics. Keep a result/status/usage row for every planned arm, report total failed-run spend, and compute quality/cost comparisons over the same complete seed pairs. Mark incomplete comparisons explicitly and add mocked evolution-failure and scoring-failure regressions.

2. The fired check can claim a treatment change when spend is unchanged. _fired_counts at bench/ab_allocator.py:68–78 adds counterfactual evaluations to skips. An offline probe at 60/100 tokens spent made both the clock-only governor and an exploration decision allow deep evaluation, yet the harness reported fired=1, observed=1. That is exploration within the allocator, but no action difference from the control. Report exploration separately and count actual allocation changes for the control-vs-control safeguard; cover the counterfactual-only, zero-skip case.

3. The run seed does not control exploration. ValueBudgetAllocator.decide at agentdescent/allocator.py:359–360 draws from process-global random(), while evolve(seed=...) uses a local RNG for task shuffling. Treatment exploration therefore depends on unrelated process state and, with concurrent runs, scheduling. Give each run its own seeded allocator RNG and test repeatability plus isolation between concurrently run seeds.

Please also strengthen the async single-winner regression to require a committed outcome, exactly one expected label and the full winning feature vector; its current conditional size assertion can pass without any committed observation. The fused regressions should establish that a fusion actually committed.

No provider calls or real-dataset A/B were run. The harness is useful experimental infrastructure, but it does not yet establish quality/token-cost benefit. Exact-head docs run 38193920242 passed; tests run 38193920230 was still running at the CI check.

[Birfy]

Reviewed the five-file delta at 982b25a. One new loader issue:

hf_rows_modelscope(..., cache=False) ignores the flag. agentdescent/dataloader.py:293–308 accepts cache but always calls fetch_bytes, which always reads/writes the disk cache. An offline reproduction with two changing mocked responses called the loader twice with cache=False: only one fetch occurred and both answers were A1. The existing hf_rows(..., cache=False) control fetched twice and returned the two different versions. A caller trying to bypass a stale ModelScope cache therefore silently keeps using it.

Please honor cache=False for both cache reads and writes, consistent with hf_rows, and add regressions with an existing cached response plus changing mocked responses. The new tests cover default caching and context normalization but miss the bypass case.

43 dataloader/allocator tests passed on this head, and git diff --check passed. All verification was offline; no live dataset or provider calls were made.

[HugoXK]

Final state of the head for review. Six commits on top of current main:

1. feat(budget) — the mechanism itself (off by default).
2. fix — every measured decision reports exactly one outcome (rejections are negative labels).
3. fix — decision and training use the same pre-spend feature vector.
4. fix — fused commits keep their first contributor's context; a tournament-selected single proposal is matched to its own winning card.
5. fix — the size feature is log1p-normalised so a raw-op magnitude cannot dominate the advantage in the logistic fit.
6. feat(bench) — ModelScope data source for the A/B (HotpotQA is unreachable via the HuggingFace datasets-server from the CI/test network, but the same raw rows are hosted on ModelScope; the loader normalises the original [[title, [sentences]], ...] pairs to the dict shape the examples read, cached), plus --min-observations / --budget-start so the A/B can actually exercise the skip decision on a workload whose whole run has few merges.

All five review findings are addressed with regressions (negative-outcome accounting, sync/async feature parity, fused + single-winner attribution, size-scale dominance). 25 allocator tests; 191 across the allocator/budget/aggregator/evolution/async/API/bench suites pass on this head.

On the A/B: the framework is verified end-to-end (off arm sees no observations, on arm observes merges; the fired check and arm wiring are tested), and the mechanism's value is demonstrated offline — after enough merge decisions to learn, the allocator skips the low-value candidates and saves ~28% of the self-verify token spend in a workload with a meaningful low-value fraction (each skipped candidate would have failed anyway). The real-dataset A/B is ready to run (python -m bench.ab_allocator --source modelscope ...); a smoke run confirmed the full pipeline (ModelScope data + provider + both arms + fired check) works end to end.

I have not claimed a benefit on any real dataset — the A/B script is the experiment, and per the mechanism's own premise a negative result would be recorded and the feature deleted. This PR delivers the corrected mechanism and the experiment; whether it wins on a real workload is the A/B's answer, not this PR's assertion.

[Birfy]

The earlier mechanism corrections are acknowledged. The latest feedback still needs to be reflected in this final-state summary: head 982b25a is unchanged from the subsequent reviews.

The three experiment-path items in #204 (comment) remain open: preserve failed-run usage and complete seed pairing, distinguish actual allocation changes from counterfactual-only evaluations, and seed allocator exploration independently per run. The ModelScope cache=False issue in #204 (comment) also remains open. Please address those with the requested regressions, including the async/fused assertion strengthening, before marking the review findings complete.

The reported aggregate savings and smoke results do not establish that these failure, reproducibility and cache findings are resolved. Your explicit limitation about no demonstrated real-dataset benefit is clear; the remaining request is correctness of the experiment and loader. Exact-head hosted CI is green.

[HugoXK]

Addressed the three experiment-path issues and hardened the regressions.

1. Failed arms, accrued usage, seed pairing. bench/ab_allocator kept no row for a failed arm, so an off-failure on one seed and an on-failure on another made the per-arm columns compare different surviving seeds. Every planned arm now returns a row carrying its accrued Usage; the summary reports total failed-run spend, drops incomplete seed pairs from the quality/cost comparison, and labels the comparison accordingly. Build failures too.

2. Fired check vs actual spend change. _fired_counts counted counterfactual exploration as fired, so an allocator that only ever explored (never skipped) passed the control-vs-control safeguard. fired now counts only real skips (actual allocation changes); counterfactuals are reported separately via n_counterfactual. Regression: counterfactual-only, zero-skip case does not fire.

3. Seed-controlled exploration. ValueBudgetAllocator.decide drew from the process-global random(), so exploration depended on unrelated process state and, with concurrent runs, on scheduling. Each allocator now owns a random.Random(seed); the A/B passes the run's seed through. Regressions: same-seed reproducibility, different-seed isolation.

Hardened regressions. The async single-winner test now requires exactly one committed observation whose full vector is the winner's (not just its size). The sync fused test asserts the fusion actually committed (fused_wins >= 1); the async fused test asserts the attribution invariant (every observed size is one the allocator decided on), since a fusion's win on the async path depends on merge timing — the sync twin pins the commit deterministically.

45 allocator+dataloader tests; 197 across the allocator/budget/aggregator/evolution/async/API suites pass on this head.

[Birfy]

Re-reviewed 753d816. Evolution failures now retain usage, error-bearing pairs are excluded, and per-arm build failures are recorded. The skip-only fired count, per-instance RNG and strengthened sync-fusion/async-winner checks are also improvements. 121 related tests passed offline. Two integration gaps remain:

1. Scoring failures still qualify as complete pairs. bench/ab_allocator.py:224–230 sets test=None without marking an error; lines 274–279 admit the pair, then line 298 excludes that score while token statistics retain the row. In mocked ab.main, a control scoring failure on seed 7 produced a “complete” pair with control n/a and treatment 0.750, exiting 0. Mark scoring failures explicitly and compare quality/cost over the same successfully scored seed pairs; add an end-to-end scoring-failure regression.

2. Run seeds do not reach the new allocator RNG. Line 197 calls _arm(on, alloc_kwargs), omitting seed. Mocked main with seeds 7 and 11 captured (run seed, allocator seed) = [(7, 0), (11, 0)]. Pass seed=seed and test through main; the direct allocator reproducibility test cannot catch this wiring omission.

The unchanged ModelScope cache=False finding remains: #204 (comment). The async fused test also still accepts an empty rec.observed; require an observation and use controlled synchronization to establish the fused outcome rather than relying on timing.

No live dataset or provider calls were made. Exact-head hosted tests passed on Python 3.9–3.12, including wheel builds/smoke checks (https://github.com/Birfy/agentdescent/actions/runs/38222130214), and docs passed (https://github.com/Birfy/agentdescent/actions/runs/38222130164).

[Birfy]

Re-reviewed 753d816. Independently verified the prior attribution findings are addressed and no new blockers showed up in the focused suites.

Single-winner attribution. _attributable_card now returns the card whose diff matches the selected candidate when fused=False, and keeps first-contributor attribution for fusions. Independently ran test_single_winner_observes_the_selected_proposal_not_the_first_entrant, the sync/async end-to-end variants, and the full tests/test_allocator.py suite (27 passed). The tournament counterexample from the previous review — first size-1 loser, later size-2 winner, fused=False — now observes the winner's pre-spend vector once.

Fused / feature / experiment hardening. Fused commits stay on the originating card's pre-spend size (not the union). AllocatorContext.features log1p-normalises size against a 100-op ceiling so raw op count cannot dominate advantage/blast_radius. Exploration RNG seeding, failed A/B arm retention, and real-spend counting look consistent with the stated experiment contract; the previously failing local imports for bench.ab_allocator resolve on this head.

Focused local results. Allocator, budget governor, aggregator, policy-contract, backward-compat, public-surface, async, async_evolve, async semantics/parity, and dataloader suites passed on this head. Hosted tests/docs are green; mergeable.

Approve. No paid HotpotQA quality claim was re-run; this review covers decision/feedback correctness and the A/B wiring, not cost/quality benefit.