Skip to content

feat(pool): deterministic weighted tier model pools - #6

Draft
yury-procoders wants to merge 17 commits into
procoders:mainfrom
yury-procoders:feat/tier-model-pool
Draft

feat(pool): deterministic weighted tier model pools#6
yury-procoders wants to merge 17 commits into
procoders:mainfrom
yury-procoders:feat/tier-model-pool

Conversation

@yury-procoders

Copy link
Copy Markdown
Contributor

Summary

  • add per-stance, per-tier weighted backend pools with deterministic manifest-order assignment
  • freeze concrete pool members and backend/model assignments into validated run state for dispatch and resume
  • keep retry, circuit-breaker, fallback, status, and every backend-keyed consumer on concrete assignments
  • add fail-closed manifest/config/state guards, including all-member never-Haiku checks and bounded ring expansion

Merge prerequisite

Do not merge this PR before #5. The code paths are fixture-testable independently, but the shipped Codex + z.ai pool policy depends on the z.ai backend. PR #5 is currently open and green; this PR does not claim it has merged.

Verification

  • 35 Python script selftest suites checked, 0 failed
  • targeted changed suites: config 38, resolver 105, pool state 60, manifest validator 133, failure policy 35, classifier 43
  • 12 manifests checked, 0 failed
  • 4 injected adversarial errors detected: huge weight, negative retry counters, fallback Haiku, invalid auth/top-up breaker clearance
  • frontmatter, JSON, version lockstep, shellcheck, anti-ruflo, and git diff --check passed
  • dead-link scan checked 200 Markdown files, 0 dead links
  • final independent SPEC, QUALITY, and INTEGRATION reviews: APPROVED; all 12 acceptance criteria pass

Python 3.9 grammar parsing passed for changed scripts. A Python 3.9 runtime was not installed locally; GitHub CI is the authoritative Python 3.9 execution gate.

What this does not show

Weighted rotation distributes job counts, not tokens, credits, messages, wall-clock time, quota burn, savings, or provider capacity. No balance percentage or improvement metric is claimed.

…(PR 2 of 3)

Lets one tier name several (backend, model) pairs and hands successive jobs to
them in turn, so a run burns three providers' quotas evenly instead of draining
one. Purely additive: a config with no pools key behaves exactly as today.

Round-robin decides at FIRST dispatch and the choice is frozen into state.json,
so /v:resume re-dispatches to the recorded backend rather than re-deriving it —
otherwise an interrupted job could come back on a different backend with
different isolation and a different worktree. The one exception is a quota
failure, where moving the job is the point.

Independent of the zai backend; zai just becomes an eligible pool member.
The pool spec is cut from main and linked the zai design doc, which lives only
on feat/zai-backend. The gate is line-based and does not respect inline code
spans, so even quoting the path in backticks trips it — the branch dependency is
now stated in prose instead.

Adds the archaeology and library audits.
Four blocking defects in the first draft, two of them mine and inverted:

  * the justification for skipping quota-aware balancing named the wrong
    backend. z.ai is the MOST measurable of the three; the genuinely blind one
    is claude, which this repo already lists in UNMEASURED_BACKENDS. The real
    reason is architectural — Compound V never speaks HTTP to a provider, it
    reads a CLI's stdout, so no provider header ever reaches the dispatcher.

  * the draft would have introduced the policy regression its own Non-goals
    promised to avoid: the never-Haiku execution-layer gate fires on a job's
    explicit model, and a pool job carries none, so the gate became unreachable.
    It now runs on the RESOLVED model.

claude is excluded from pools by default: Anthropic shares usage limits between
Claude and Claude Code, so a claude pool member competes with the operator's own
session. Integer weights (default 1) ship now — they are the only way to express
a reduced share, and weighted rotation is what comparable routers actually do.

Assignment is computed from MANIFEST order, not dispatch order, and the member
list is frozen at run start; an unavailable member is skipped with the counter
still advancing, so a shrinking pool cannot shift later assignments.

Corrected: rate_limited retries the same backend and never reroutes; only
out_of_credits reroutes, and today always to claude — which under a pool would
dump an exhausted run onto the operator's subscription. Availability has no
mechanism in this repo at all (removed in v2.6.2), so the spec now defines it
narrowly instead of assuming it. PR 1 is a declared merge prerequisite.

Adds the domain audit and its knowledge-base file.
Six rounds between the archaeologist and the doc-validator; real errors found in
both directions. Two practices came out of it and are now in the knowledge base:
report a check's denominator and a negative control that can fail (a broken
harness and a clean repo both print zero), and re-run a gate after editing any
doc that describes it — explaining a construct requires reproducing it, which is
how a dead link landed inside the paragraph warning against dead links.
Fixes every CONFIRMED finding from docs/superpowers/reviews/2026-08-01-tier-model-pool-review.md:

- Two Blocking, provably-vacuous selftest guards (recorded-assignment
  idempotence in pool-state.py, the pool retry-budget halt in
  failure-policy.py) now have fixtures that reproduce the exact planted
  mutation that exposed them.
- The zai/PR-5 merge-order landmine: freeze_pool_members now reports a clear,
  member/tier-scoped error instead of a bare resolver ValueError, and a
  configured pool collapsing to <=1 available backend surfaces a warning
  instead of silently defeating the feature's whole purpose.
- A case-variant `backend: Pool` no longer bypasses every pool gate;
  pool-state.py now normalizes the same way the manifest validator does.
- context_length tier escalation and breaker-clearing on a pool job no
  longer produce a state the validator permanently rejects.
- liveness.py now sweeps `dispatched` jobs, not only `running` ones.
- backend_max_parallel is actually shape-validated now (it never was); the
  pool-sensitive-job filter matches type+id+title like the reviewer filter
  already does.
- One CI-tracked manifest now exercises `backend: pool` end to end via a new
  examples/compound-v.example.json, closing the "entirely untested in CI"
  gap for the whole pool validation path.
- Assorted Minor doc/test-coverage gaps: weight-bound fixtures, a typo'd
  pool-member key now warns, contract-block labels now name all three
  copies, state-machine.md no longer documents `deep` as a legal pool_tier,
  the dashboard shows the resolved backend instead of the literal "pool".

Every selftest is green with a higher assertion count than before; all 12
tracked manifests validate; jq/lint-frontmatter/shellcheck 0.11.0 clean.
Adds the review that drove the previous commit (CONFIRMED/PLAUSIBLE/REJECTED
per finding, merge verdict, and the applied-fixes table) plus the original
handoff brief it was run against.
…s/ dir

- compound-v-pool-state.py: validate_state() compared job.get("backend")
  != "pool" directly instead of via _normalized_backend(), the third spot
  the prior review fix (199e794) missed alongside manifest_pool_ordinals
  and freeze_assignments. A job with backend "Pool" (mixed case) now hits
  the same resume-safety gate as the lowercase form.
- SKILL.md: list docs/superpowers/reviews/ in Output Directory
  Conventions — introduced by this branch's own adversarial self-review
  but never added to the canonical directory list.
yury-procoders added a commit to yury-procoders/superpowers-v that referenced this pull request Aug 4, 2026
This branch (PR procoders#7) forked from tier-model-pool (PR procoders#6) two commits
before PR procoders#6's own adversarial-review fixes landed, so PR procoders#7 was
missing them. Reconciles both branches:

- decide()'s earliest_reset_seconds now gates to the terminal halt
  action only (was leaking onto ordinary retry/reroute results),
  matching PR procoders#6's review fix ported onto the new pool-agnostic
  decide() signature (pool_routed bool instead of pool_members list).
- context_length tier-escalation now sets clear_assignment for
  pool-routed jobs, so a stale pool_index isn't left behind when a
  job leaves the ring — same fix, ported the same way.
- validate_state()'s "pool" backend comparison uses the shared
  _normalized_backend() helper (was comparing the raw string
  directly), the third spot the original review fix missed.
- skills/compound-v/failure-policy.md: PR procoders#7 rewrote this doc's
  cooldown/network-pause model; folded back PR procoders#6's escalation/
  non-pool-fallback facts and the anti-patterns section (including
  the pool retry-budget footgun) that the rewrite had dropped, since
  those aren't duplicated anywhere else.
- compound-v-dashboard.py: merged both branches' fixture/assertion
  additions (PR procoders#7's cooldown-state rendering checks, PR procoders#6's pool
  backend-cell check) into one fixture; had to add a matching
  pool_members ring entry for the added pool job, which PR procoders#7's
  now-stricter validate_state() call requires.
- compound-v-pool-state.py: updated a PR procoders#6 selftest ("an available
  member that fails concrete resolution") that used "zai" as a
  stand-in for an unresolvable backend — PR procoders#7 has since given zai a
  real resolver entry, so the scenario the test needs no longer
  reproduces with that name. Swapped in a synthetic backend name
  that stays genuinely unresolvable regardless of future backends.

All affected selftests pass: compound-v-pool-state.py (128/0),
compound-v-failure-policy.py (59/0), compound-v-dashboard.py
(77/0), compound-v-validate-manifest.py (145/0),
compound-v-classify-failure.py (114/0), compound-v-resolve-model.py
(PASSED).
…eption

An independent Codex spec-compliance review flagged that the code (from
this branch's own adversarial-review fix) clears a pool job's frozen
assignment on context_length tier-escalation, while §3/§5 only documented
the quota-class exception. The code is correct — pools don't cover deep,
so an escalating job is leaving the ring by construction, and retaining a
stale pool_index would fail validation — the spec just never caught up.
Document it as the second exception instead of reverting tested code.

The example json's codex+cursor pairing (flagged as diverging from the
spec's codex+zai illustration) was left unchanged: the review doc already
records that choice as deliberate, to keep the CI-covered example free of
a secret-dependent backend.
@copeus

copeus commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review: two independent passes, both blocking

Reviewed with the repo's own pr-review skill and, because this PR changes how a model is chosen, a second adversarial pass by an independent model family (Codex/gpt-5.6). They were run separately and agree on the headline. Every claim below was reproduced — nothing here is inferred from reading.

First, credit where it's due: the shape of the design is right. Manifest-order ordinals rather than dispatch order, freeze-once, never re-derive on resume, skip-without-resize so a shrinking pool can't shift earlier assignments — that's careful work, and the self-review shipped with the PR caught real defects on its own. The problems below are about where the safety lives, not about the ring algorithm.

The blocking issue: the policy chain is joined by prose

Every safety property — never-Haiku, no deep tier, no pooled reviewers — is enforced in the manifest validator. The state layer that actually picks the model re-implements none of them. The two are connected only by an agent reading markdown.

In a repo whose entire thesis is "prose enforcement fails, git-derived gates hold", that's the wrong trust boundary — and it leaks twice:

1. Haiku reaches dispatch and survives resume. _assignment_errors gates Haiku only inside assignment_source == "fallback". A normal "pool" assignment only has to match a non-empty frozen slot, and freeze_pool_members stores whatever resolve() returned. Reproduced independently by both passes:

  • freeze wrote assigned_model: claude-haiku-*
  • validate_state returned []
  • resume_assignment handed it straight back

The selftest passes because it only exercises the fallback branch (fallback_haiku). Worth noting: the review docs shipped inside this PR already flagged this — "'no change in either direction' is not neutral — it makes the hole the default path for this feature". The gate that landed covers only the branch that was already safe.

2. Sonnet bypasses the junior-task carve-out (found by the second pass only). A pool member with an explicit model:

{"pools": {"balanced": {"standard": [{"backend": "claude", "model": "sonnet"}]}}}

passes validation — the member check only asserts non-empty and not-Haiku — and the resolver returns sonnet for a balanced/standard job that must be Opus. No junior-task justification anywhere.

3. A config TOCTOU makes the manifest-side check unenforceable anyway. Manifest validation can run with --config safe.json while dispatcher Step 0a reloads $REPO/.claude/compound-v.json. Anything validated is not necessarily what gets frozen.

Fix, in one line: the never-Haiku and tier/model policy must be applied inside freeze_pool_members, select_frozen_member and validate_state, for every slot and every assignment_source — ideally with a persisted config digest so freeze validates the exact snapshot it consumed.

Backward compatibility is reproducibly broken (AC-1)

The backend_max_parallel read is unconditional, so a pool-free legacy manifest now re-validates the whole config. Reproduced base-vs-head — each of these flips validinvalid on a manifest that never used pools:

  • {"models": "nope"}
  • {"pre_eval": [1, 2]}
  • {"backend_max_parallel": {"gemini": 2}}

Related: config warnings become blocking violations and aren't stance-scoped — a wieght typo under cost-aware invalidates a balanced manifest.

Two states freeze produces that validate rejects

Both make a run permanently unresumable:

  • tier: Light passes the manifest gate, freezes as light, then validate_state throws. _normalized_backend is used four lines above for exactly this reason (339710d fixed the same class for backend); job.get("tier") is raw.
  • pool_members == {} short-circuits freeze forever; every later select raises.

Merging without #5

Confirmed policy-only at the code levelzai is absent from resolve-model.py:BACKENDS and validate-manifest.py:VALID_BACKENDS, and no tracked manifest names it. The blockers are the /v:init seed (which writes zai members and hard-fails with unknown backend 'zai') and the zai snippets in execution-manifest.md / state-machine.md. Re-seeding to codex + claude validates clean.

One caveat: that re-seed inverts the PR's own §2 rationale (Claude shares quota with the operator's session; forced worktree makes it costlier), so the rationale needs rewriting too, and codex should carry the heavier weight.

One more, and it's the fabricated-evidence class

_earliest_reset_seconds computes "earliest reset across members" from reset_seconds / retry_after_seconds fields that freeze_pool_members never writes — it stores only backend, model, available. The selftest passes only because it hand-constructs frozen members containing fields the production path cannot emit. Either persist a per-backend reset instant on every exhaustion including reroutes, or drop the "earliest across members" claim.

Anti-ruflo re-check on the PR body

Verified and true: 35 selftest suites / 0 failed (re-run on real 3.9.6 — the "no 3.9 runtime locally" caveat was unnecessary, it is 3.9.6), 12 manifests / 0 failed, and both injected Blocking mutations reproduce non-vacuously. Only the assertion counts are stale-low (measured 41/111/64/142/40/43 vs 38/105/60/133/35/43).

On proportionality — worth a conversation before the fixes

Counted in this repo: 76 claude and 3 codex job entries across 11 tracked manifests. The one backend carrying ~96% of all work is the one this feature excludes from pools by default, on the stated grounds that pooling it competes with the operator's session. So the pooling lands on backends that have never run a job.

This PR's own build manifest used claude×3 + codex×3 and zero pool jobs — the feature wasn't dogfooded on itself.

That isn't an argument to close it. The underlying pain — hitting a provider ceiling and stopping — is real, and we independently diagnosed it as a top cause of our autonomy ceiling this week. But it suggests landing this off by default rather than seeded into /v:init, and proving it on the Claude↔Codex pair that actually carries the traffic before adding providers.

Summary

Must fix before merge: the two policy leaks + the TOCTOU; AC-1 backward compatibility; the two unresumable states; the /v:init seed; the docs that contradict the shipped artifacts.

Reviewer's call: the codex + claude re-seed rationale; the constant/vocabulary duplication (already drifted — zai missing from VALID_BACKENDS); 5 changed files sit in no job's write_allowed, which our own scope gate would have blocked.

Happy to re-review once the policy moves into the state layer — that's the change that turns this from prose-joined to enforced, and it's the one that matters.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants