Repository navigation
feat: separate who reviews a session from the harness that holds it - #165
Conversation
STRML
left a comment
STRML
left a comment
There was a problem hiding this comment.
The direction is right, and I want this in. A Codex review pass on 4293ff5 found six problems that need fixing before merge, mostly where the new engine choice meets state that was keyed by source before.
| # | Problem | Where |
|---|---|---|
| 1 | AUTODREAM_L1_MODEL_<NAME> is still keyed by source, but a model id belongs to an engine |
bin/adapters.sh adapter_l1_model, bin/run.sh source loop |
| 2 | AUTODREAM_L1_REQUIRE_SERVED=1 only checks fresh calls; cached findings and reused chunk answers skip it |
dispatch_l1: the idempotency read, the .chunkout reuse |
| 3 | The chunk cache digest has source and model but not engine | dispatch_l1, the cfg digest |
| 4 | A bad engine override or failed login writes a no L1 engine record that counts as done, so fixing the config never retries it |
source loop + login loop in run() |
| 5 | served compares the requested string to the canonical id, so haiku vs claude-haiku-5-5 is a mismatch |
l1_served_mismatch, the REQUIRE_SERVED check |
| 6 | An engine with no served verb (omp) adds nothing to l1_cost_usd_estimate, so an all-omp night reads 0.0000 |
run-stats block |
Repro for 1: claude as reviewer with AUTODREAM_L1_MODEL_OMP=deepseek/deepseek-flash sends the omp sessions to the claude CLI with --model deepseek/deepseek-flash. Going the other way, an omp pin never applies to claude sessions routed to omp. Either key the model by engine, or refuse a source pin that resolves to a different engine.
For 4, the "deterministic, leave it in place" rule in dispatch_l1 held when the only cause was a missing adapter on disk. A config or login refusal can change between runs, so those records need to be retried (or not written at all) once the engine resolves.
For 6, CLAUDE.md's rule is that a degraded measurement says so and never reads as zero. Count the calls with no cost and print that next to the sum, e.g. l1_cost_unmeasured: 14.
One heads-up from the host side: with claude,omp enabled and AUTODREAM_L2_ENGINE=omp, this PR moves every Claude Code session from Haiku to omp's default model. I've pinned AUTODREAM_L1_ENGINE_CLAUDE=claude on my install so that doesn't happen silently, but anyone else running both harnesses gets the move on upgrade. A line about it in the CHANGELOG would cover it.
An adapter other than claude only read its manifest's default roots. A harness that keeps one session store per profile, as omp does under each profile's agent/sessions, was invisible: the default root can sit empty for weeks while every live session is somewhere else. AUTODREAM_SESSION_ROOTS_<NAME> (colon-separated) replaces the manifest defaults for that one adapter, the way SESSION_ROOTS works for claude.
Which harness holds a transcript and which engine reviews it were the same choice: each adapter's sessions went to its own engine in L1. With omp enabled, omp sessions could only be reviewed through omp and whichever provider its model routes to. They are now separate. A run has one reviewer, the engine that writes the summary (AUTODREAM_L2_ENGINE, else the first enabled adapter), and by default it reviews every source's sessions in L1 as well. AUTODREAM_L1_ENGINE picks another engine for L1, and AUTODREAM_L1_ENGINE_<NAME> one for a single source. The source's own adapter still enumerates, linearizes, describes and adds its triage notes; only the model call, its warmup and the network probe move. The default L1 model comes from the engine's manifest. A host with one harness behaves exactly as before. A host with several now has them all reviewed by one engine unless it asks otherwise, and AUTODREAM_L1_ENGINE_<NAME>=<name> keeps a harness reviewing its own. The engine is resolved in the parent and exported beside the source and model maps, so a worker cannot change which binary the next one runs. A choice that names no accepted adapter fails closed: those sessions get error records and no model call. run-stats records l1_engine_<source>. Tests: tests/adapters.sh covers the resolution order. In run-all, the reviewer reads omp sessions while a tripwire omp binary records no calls, AUTODREAM_L2_ENGINE moves both passes whatever the adapter order, AUTODREAM_L1_ENGINE moves every source, and an unusable choice starts no worker. The existing omp tests pin AUTODREAM_L1_ENGINE_OMP=omp so they keep testing the omp engine.
A run named the models it asked for and nothing else. Nothing showed
which account or API the reviewing engines were signed in to, or which
model actually answered each L1 call, so a report could not be traced
to the model behind it and a substituted model would go unnoticed.
Each engine the run uses, the L2 engine and every L1 engine, reports
its login once through a new adapter verb, whoami: API provider,
sign-in method, plan and account. For claude that is
`claude auth status --json`, which makes no model call. It is logged
and written to run-stats as login_<engine>.
The claude L1 worker now runs with --output-format json, and the new
served verb reads the models that answered from the result object.
Every call is recorded in findings/<date>/l1-served.tsv and summed in
run-stats (l1_served_models, l1_served_mismatch, and
l1_cost_usd_estimate, the CLI's estimate at API prices).
Both are record-only unless the host asks:
AUTODREAM_REQUIRE_LOGIN="provider=firstParty auth=claude.ai plan=max"
an L1 engine whose login does not match is handed no transcript,
through the same error-record path as an unusable engine choice; an
L2 engine that does not match stops the run before it starts
AUTODREAM_L1_REQUIRE_SERVED=1
an L1 call answered by any model other than the requested one loses
its findings, so nothing a substituted model wrote reaches L2
Not stream-json: --verbose echoes every Read result, the whole
transcript, onto stdout, and a failed worker copies its stdout into the
.err file L2 reads.
AUTODREAM_L1_MODEL_<NAME> was still read for the source after the engine choice was split from it, but a model id belongs to an engine. With claude as the reviewer and AUTODREAM_L1_MODEL_OMP set to a deepseek model, omp sessions went to the claude CLI with --model deepseek/...; going the other way, an omp pin never reached claude sessions routed to the omp engine. adapter_l1_model now resolves the engine first and reads AUTODREAM_L1_MODEL_<ENGINE>, then AUTODREAM_L1_MODEL, then that engine's manifest default, the same keying AUTODREAM_L2_MODEL_<NAME> already has. The engine choice stays per source. Tests: tests/adapters.sh asserts a source pin no longer reaches another engine and an engine pin reaches every source it reads; run-all checks the worker argv for the review repro in both directions.
A chunk answer is reused only for the same chunk read under the same instructions, and the digest in its cache name said so for the source and the model but left out the engine. Now that the engine is a choice of its own, one model id can be served by two engines with different system prompts and tools, so a chunk answered on one engine was reused after the session moved to another. Tests: run-all leaves a chunk answer behind on the claude engine, moves the session to the omp engine under the same model id, and checks the chunk is called again rather than reused.
The served check compared the requested string to the id the CLI reports. The CLI reports canonical ids, so a host that asked for haiku saw every call answered by claude-haiku-5-5 counted as a mismatch, and AUTODREAM_L1_REQUIRE_SERVED=1 discarded all of them. The served verb now takes the requested model and returns a verdict, match or mismatch, beside the models and the cost. Only the adapter knows its CLI's naming: for claude a bare family name matches any id of that family (claude-<name>-...), a full id only itself, and a context suffix such as [1m] is ignored. The verdict is a sixth column in l1-served.tsv, and both the REQUIRE_SERVED check and l1_served_mismatch read it, so the two cannot disagree. An engine with no served verb (omp) gets the verdict unmeasured. It was counted as a mismatch before; it is now counted on its own line, l1_served_unmeasured, and REQUIRE_SERVED=1 still discards its calls, with a message saying the engine reported no model. Tests: tests/adapter-claude.sh covers aliases, full ids, prefixes, suffixes and unreadable aliases. run-all checks that a haiku request answered as claude-haiku-5-5 keeps its findings under REQUIRE_SERVED, and that an omp call is unmeasured and discarded under it.
The served check ran only on fresh calls. A findings JSON left by an earlier run is reused by the dispatcher's idempotency read, and a finished chunk answer by the chunk cache, so with the requirement turned on after a night without it, or added on an install whose findings predate it, a substituted model's answer reached L2 anyway. The runner now stores the verdict beside every answer it accepts, as <answer>.served (engine, requested model, models that answered, verdict), after the engine has exited. A merged session is match only when every chunk was. Before L1, drop_unacceptable_l1_cache removes any cached findings JSON or chunk answer whose stored verdict is not match, or that has none, so it is triaged again. It runs in the parent before the pre-L1 snapshot, so the dispatcher, l1_missing_count and l1_sessions_already_done_at_start see the same set. Records carrying error or skipped are the runner's own and are kept. run-stats gains l1_cache_dropped. Tests: run-all reruns a date under the requirement and checks that findings another model wrote, and findings with no stored verdict, are triaged again, that matching ones and runner stubs are reused, and that a cached chunk answer is redone or reused by the same rule.
A session with no usable L1 engine gets an error record that counts as done, on the rule that retrying cannot change the answer. That held when the only cause was an adapter missing on disk. Now an engine choice naming no accepted adapter, a missing model or a login failing AUTODREAM_REQUIRE_LOGIN writes the same record, and all of those are config: fixing it and running the date again (a catch-up or AUTODREAM_FORCE=1) still skipped the session. The record now carries no_engine: true. The pass that runs before L1, drop_unacceptable_l1_cache, drops it when this run resolves an engine and a model for the session's source, read from AUTODREAM_SOURCE_MAP rather than the record, so the session is triaged. Within one run the record still stands, so the retry rounds do not spin on it and the report still ships. Records written before the marker are recognised by their error text. Tests: run-all writes the record with a bad engine choice and with a failing login, then reruns the date: the same bad config keeps it and calls nothing, a fixed one triages the session.
l1_cost_usd_estimate sums the cost each engine reports, and an engine with no served verb (omp) reports none, so an all-omp night read 0.0000: a degraded measurement that looks like a real one, which the self-audit rule in CLAUDE.md forbids. run-stats now prints l1_cost_unmeasured beside the sum, the number of calls it leaves out, so a zero reads as "nothing measured" when it is. Tests: run-all checks l1_cost_unmeasured is 0 on a claude night and 1 on a one-session omp night whose estimate is 0.0000.
…fixes Includes the upgrade note for a host running both harnesses with omp as the L2 engine: the default reviewer moves its Claude Code sessions from Haiku on the claude CLI to omp's L1 model unless it pins AUTODREAM_L1_ENGINE_CLAUDE=claude.
4293ff5 to
ed8be43
Compare
|
Thanks for the careful pass. All six are fixed, one commit each (5 lands before 2, because 2 stores 5's verdict), on top of a rebase onto current main (#163 and #164 landed first, so I re-registered the new tests with
On trust: The CHANGELOG entry (ed8be43) leads with your heads-up: with Full suite: 3,078 passed, 0 failed (four shards, plus |
STRML
left a comment
STRML
left a comment
There was a problem hiding this comment.
Five of six are fixed. A second Codex pass on ed8be43 confirmed 1, 3, 4, 5 and 6, and found one hole left in 2.
l1_record_is_runner_only (bin/run.sh, the has("error") or has("skipped") check) trusts the key names, but a worker holds the Write tool and can put either key in its own answer. Repro:
- Run a date without
AUTODREAM_L1_REQUIRE_SERVED. A substituted model writes{"skipped":true,"findings":[{"what":"incorrect"}]}, and its.servedsidecar saysmismatch. - Defer or interrupt the run, set
AUTODREAM_L1_REQUIRE_SERVED=1, rerun. drop_unacceptable_l1_cacheseesskipped, keeps the file without reading the verdict, and the idempotency check in the worker accepts the findings array. It reaches L2.
Every record the runner writes itself has findings: [], so the predicate can require that: (has("error") or has("skipped")) and (.findings == []). A record with findings in it is model output whatever else it carries, and goes through the verdict. A test with the payload above would pin it.
That's the last one. Once it's in I'll merge.
drop_unacceptable_l1_cache kept any cached findings JSON carrying
error or skipped without reading its stored verdict, on the grounds
that the runner writes those records itself. A worker holds the Write
tool and can put either key in its own answer, so under
AUTODREAM_L1_REQUIRE_SERVED=1 a substituted model's
{"skipped":true,"findings":[...]} from an earlier run was kept, passed
the worker's idempotency read and reached L2. The no-engine marker had
the same gap: a forged no_engine record with findings was kept for as
long as its source had no engine.
Every record the runner writes has findings: [], so both predicates
now require it. A record with findings in it is model output whatever
else it carries and goes through the verdict.
Tests: run-all forges skipped, error and no_engine answers with
findings over a mismatched verdict, reruns under the requirement and
checks each is dropped and triaged again (or replaced by the runner's
own no-engine record when the source has no engine).
|
Fixed in e142066 with your predicate, The no-engine check had the same hole, so it gets the same guard. A worker could write The test uses your payload. Run 1 leaves findings under a Full suite: 3,087 passed, 0 failed. |
STRML#165 was squash-merged upstream as 2875df0 after two review rounds, and STRML#168 (82fdd83) closed the last two cache-verdict holes on top of it. The fork's only content beyond the shared base was the three pre-review STRML#165 commits from PR #2, all superseded by the reviewed version, so every path takes upstream's: the tree is upstream's main.
What this does
autodream reviews coding sessions from more than one harness, such as Claude Code and omp. Until now, the harness that held a session also decided who reviewed it, so omp sessions could only be read by omp's own engine and whichever provider its model pointed at.
This PR separates the two. A run has one reviewer, an engine and a model, and every harness's sessions go to it. Each harness still does the part only it can do: it finds its sessions, reads them into the common shape and describes them.
It also records who the reviewer is signed in as and which model answered each call, and lets a host refuse to run when either one is not what it expects.
flowchart LR claudeSessions["Claude Code sessions"] ompSessions["omp sessions"] reviewer["The run's reviewer<br/>one engine and one model"] findings["Findings for each session"] summary["Summary report<br/>written by the same reviewer"] claudeSessions --> reviewer ompSessions --> reviewer reviewer --> findings findings --> summaryWhat changes for existing setups
With one harness enabled, nothing changes: its engine reviews its sessions and writes the summary, as before.
With several enabled, they are now all reviewed by one engine, the one that writes the summary (
AUTODREAM_L2_ENGINE, else the first enabled harness).AUTODREAM_L1_ENGINE_<NAME>=<name>keeps a harness reviewing its own sessions.So a host with
claude,ompandAUTODREAM_L2_ENGINE=ompmoves its Claude Code sessions from Haiku on the claude CLI to omp's L1 model on upgrade, unless it setsAUTODREAM_L1_ENGINE_CLAUDE=claude. The CHANGELOG says so.AUTODREAM_L1_MODEL_<NAME>now names the model of one engine, not one harness, because a model id belongs to an engine. A host whose harnesses each review their own sessions sees no change.Settings
AUTODREAM_L2_ENGINE(existing)AUTODREAM_L1_ENGINEAUTODREAM_L1_ENGINE_<NAME>AUTODREAM_L1_ENGINEAUTODREAM_L1_MODEL,AUTODREAM_L2_MODEL(existing)AUTODREAM_L1_MODEL_<NAME>,AUTODREAM_L2_MODEL_<NAME>(existing)AUTODREAM_SESSION_ROOTS_<NAME>AUTODREAM_REQUIRE_LOGINprovider=firstParty auth=claude.ai plan=maxAUTODREAM_L1_REQUIRE_SERVED1discards findings from any call the engine cannot vouch was answered by the requested model, and cached ones from earlier runs that no stored verdict vouches forHow the recording works
Before the run hands anything to an engine, the engine reports who its calls run as, through a new adapter verb,
whoami. For Claude Code that isclaude auth status --json, which makes no model call. The answer (provider, sign-in method, plan and account) goes in the log and in run-stats aslogin_<engine>.Each per-session call now runs with
--output-format json, and the newservedverb reads which models answered from the result and gives a verdict,matchormismatch. The adapter judges it because only it knows the CLI's naming:haikuanswers asclaude-haiku-5-5. Every call is recorded infindings/<date>/l1-served.tsvand summed in run-stats asl1_served_models,l1_served_mismatch,l1_served_unmeasured(an engine that reports no model, such as omp),l1_cost_usd_estimateandl1_cost_unmeasured(calls the sum leaves out). The cost is the CLI's estimate at API prices, not a bill. The worker uses JSON rather than stream-json because--verboseechoes every file the model reads, including the whole transcript, onto stdout, and a failed worker's stdout ends up in the.errfile that the summary reads.When a per-session engine fails
AUTODREAM_REQUIRE_LOGIN, its sessions get error records and no model call. When the summary engine fails it, the run stops before it reads anything. WithAUTODREAM_L1_REQUIRE_SERVED=1, findings from a call whose verdict is notmatchare discarded, so they never reach the summary.The verdict is stored beside each accepted answer. Before per-session review starts, the run drops cached findings and chunk answers from earlier runs that no
matchverdict vouches for (l1_cache_dropped). The same pass drops a no-engine error record once the run resolves an engine for that session's harness. A bad engine choice or a failed login is config, so fixing it and rerunning the date now triages those sessions. A cached record is treated as the runner's own (an error or a skip, kept without a verdict) only when its findings array is empty, because a worker can write any key but every record the runner writes hasfindings: [].Commits
Review fixes, one per point (the review's number in brackets; 5 comes before 2 because 2 stores 5's verdict):
Second review:
Testing
tests/run-all.sh, rebased on currentmainand run as four shards: 3,087 passed, 0 failed.tests/shard.shpasses.bin,adaptersandinstall.shat warning;testsat error) are clean.AUTODREAM_L2_ENGINEmoves both passes whatever the harness order.AUTODREAM_L1_ENGINEmoves every harness's sessions, Claude Code's included. An engine name that is not an accepted adapter starts no worker. The existing omp tests pinAUTODREAM_L1_ENGINE_OMP=omp, so they still test the omp engine.--model haikuand--model claude-haiku-5-5(no transcript sent): the CLI reportedclaude-haiku-5-5for both, andservedgavematchfor either request andmismatchforsonnet.🤖 Generated with Claude Code