Skip to content

fix: record concurrency as a run condition, not a plan field - #3

Merged
KimmoZAG merged 1 commit into
mainfrom
fix/concurrency-run-condition
Nov 5, 2026
Merged

KimmoZAG merged 1 commit into
mainfrom
fix/concurrency-run-condition

Conversation

@KimmoZAG

@KimmoZAG KimmoZAG commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Problem

contract.generation.concurrency declared something a plan cannot enforce:

  • prepare wrote a hard-coded 1 (there was no flag), while run defaults to 16, so the declared value
    was false for the standard flow documented in README.md.
  • run appended the same deviation string on every run --
    "concurrency differs from contract.generation.concurrency" -- without comparing anything and without
    naming either value. That is noise, not a record.
  • Because the field lived in contract.json, contract_sha256 changed when only the client's scheduling
    changed, which weakens "same contract hash <=> same evaluation definition".

Change

Concurrency is a property of a run, not of the evaluation, so it moves to the run record.

  • plan: drop concurrency from contract.generation; prepare --concurrency is removed. contract.json
    again describes only what is evaluated (prompts, gold answers, limits, scorer) and what a request must carry.
  • run: record concurrency plus an optional expected_concurrency. --expected-concurrency N is opt-in
    (default: not checked); a differing --concurrency is written to deviations as a structured
    {field, expected, actual, source} entry instead of free text.
  • run: meta files are now run.<ts>.<seq>.json. Previously two invocations inside the same second
    overwrote each other, which could silently drop a resumed invocation's record.
  • run: a resumed invocation that changes concurrency is always recorded, whether or not a target was given --
    one run directory must not silently mix two run conditions.
  • run: concurrency < 1 is rejected instead of failing inside ThreadPoolExecutor.
  • summary: read every run.*.json, merge deviations and report invocations; previously only the first
    meta was read, which would have hidden the resumed drift.

Verification

PYTHONPATH=src:tests python3 -m unittest discover -s tests
Ran 47 tests in 4.236s
OK
  • New tests: no target -> deviations == []; mismatch -> exact structured entry; match -> empty; resume drift
    -> both invocations keep their own meta and summary reports both; concurrency=0 rejected.
  • Backward compatible: a plan frozen before this change (12032 tasks) still loads; run reads only
    temperature and seed from generation, so the stale key is inert.

Note

This replaces the earlier approach of writing the target into the contract. It keeps the useful part -- the run
records the condition it ran under, and drift stays detectable -- without making contract_sha256 depend on how
many requests a client had in flight.

contract.generation declared a concurrency it could not enforce: prepare wrote a hard-coded 1,
while run defaults to 16, and run emitted the same deviation string on every run, naming neither
value. A frozen plan cannot decide how many requests a client keeps in flight, so the field is
removed from the plan and the run records the condition it actually ran under.

- plan: drop concurrency from contract.generation and remove `prepare --concurrency`; contract_sha256
  is again a statement about the evaluation alone (prompts, gold answers, limits, scorer)
- run: record `concurrency` plus an optional `expected_concurrency` (default: not checked); deviations
  become structured {field, expected, actual, source} and appear only when a target was requested or a
  resumed invocation changes concurrency
- run: name meta files run.<ts>.<seq>.json so two invocations in the same second no longer overwrite
  each other (this used to silently drop the resumed invocation's record)
- summary: merge deviations across all invocations and report `invocations`
- docs and tests follow; 47 tests pass
@KimmoZAG
KimmoZAG merged commit 06ee3a3 into main Nov 5, 2026
2 checks passed
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.

1 participant