[cosminacho]

Adds simulation.turns for authored prompts sent verbatim, in order, in one agent session, without an LLM user. Tasks can disable the working-directory prefix when exact prompt text matters. Dataset values expand in each turn; execution artifacts retain the authored dialog and raw agent-limit facts.

A script owns its dialog length (1–100 turns), so max_turns is derived from the list. Unsupported variant prompt replacements and mutations fail before changing a scripted task. Existing simulated-user defaults and agent run limits stay unchanged.

Validation: 138 directly affected test cases passed across guarded runs; the existing CLI case used an owned empty experiment and disabled Python provisioning. Nineteen independent resolver/run/reload controls passed. Changed production files passed type checking; Ruff, prose checks, and applicable architecture rules passed. No full-suite claim.

[cosminacho]

🤖 I am AI Agent Claude Code (Nacho) Opus 5.5 responding on behalf of Cosmin Maria.

CI status on ffa1713: everything passes except Delegate Live E2E (ROPC), which fails before any test runs at "ROPC login — mint a fresh Delegate access token" with ROPC token request failed: invalid_client. It fails the same way on main (push run on 2026-10-30) and on the recent Dependabot PRs, so the ROPC client secret behind that job likely needs rotating.

• 1565ea3 fixes the CodeQL alert.
• ffa1713 bumps pyasn1 to 0.6.5 for four High advisories that failed the OSV scan on main as well. It's lockfile-only and happy to move into its own PR if you prefer.

[github-actions]

Claude finished @cosminacho's task in 1m 23s —— View job

---

Code Review in Progress

Todo List:

• Read .github/code_review.md for review standards
• Read CLAUDE.md for project conventions
• Run git diff origin/main...HEAD to see full changes
• Review each changed file with full context
• Perform cross-file consistency checks
• Analyze "what's missing"
• Provide design-level scrutiny
• Post comprehensive review feedback

[uipreliga]

Review: coder_eval — pr:246 (22 files) axis:1,2,3,4,5,6,7,8

Scope: pr:246 (22 files) axis:1,2,3,4,5,6,7,8 · branch feat/scripted-dialog-turns (pr-246) · ffa1713 · 2026-11-05T14:21Z · workflow variant

Change class: complex — adds a new schema field family (simulation.turns, working_directory_prefix), a new stop reason, and changes orchestrator dialog control flow and execute/grading gating

PR 246 adds scripted user turns with clean security, typing and error handling, but it puts every scripted dialog on paths that can give a wrong result: a FinalStatus can be wrong, dataset rows can get unrendered prompts, variant prompt changes can be silently ignored and recorded configs can fail to reload. Fix the two high-severity Evaluation Harness defects and the prompt-override gap before merge.

Summary

Axis	Score	🔴	🟠	🟡	🔵	Top Issue
1. Code Quality & Style	8.8 / 10	0	0	2	2	PR raises complexity of the hottest dialog driver: _simulation_dialog_loop F(41) to F(44), _evaluation_loop D(23) to D(24), partly through an optional user parameter with a fallback built inside the loop
2. Type Safety	9.9 / 10	0	0	0	1	persona/goal loosened to `str
3. Test Health	9.2 / 10	0	0	1	3	The ungraded-dialog backstop that this PR narrowed has no test on either side
4. Security	10 / 10	0	0	0	0	—
5. Architecture & Design	9.2 / 10	0	0	1	3	The 'one source of truth for the first prompt' invariant is enforced only when TaskDefinition is built. The variant prompt layer bypasses it, so a variant initial_prompt is silently ignored and the recorded config can no longer be reloaded
6. Error Handling & Resilience	9.9 / 10	0	0	0	1	ScriptedUser's documented IndexError (a first-party cursor invariant) is swallowed by the simulator's blanket except and logged as a simulator failure
7. API Surface & Maintainability	9 / 10	0	0	2	0	simulation.max_turns is a redundant, must-set second source of truth for scripted dialog length
8. Evaluation Harness Quality	7.4 / 10	0	2	1	1	Scripted dialog loses agent max_turns_exhausted on final turn; row classified FAILURE not MAX_TURNS_EXHAUSTED

Overall Score: 9.2 / 10 · Weakest Axis: Evaluation Harness Quality at 7.4 / 10
Totals: 🔴 0 · 🟠 2 · 🟡 7 · 🔵 11 across 8 axes.

Blockers

1. [Axis 8] Scripted dialog loses agent max_turns_exhausted on final turn; row classified FAILURE not MAX_TURNS_EXHAUSTED (src/coder_eval/orchestrator.py:2608) — In _simulation_dialog_loop, the evaluate_stop break (L2602-2611: stop_decision = evaluate_stop(...) / if stop_decision.stop: ... stop_reason = stop_decision.reason / break) runs BEFORE the fact recording at L2618-2619 (if turn_record.max_turns_exhausted: / self.result.max_turns_exhausted = True). The new precedence entry at termination.py:72-73 (if config.turns is not None and turns_completed >= len(config.turns): return StopDecision(stop=True, reason=DialogStopReason.SCRIPT_EXHAUSTED)) makes this break the NORMAL exit of every scripted dialog. As a result, when the agent exhausts its inner max_turns on the final scripted turn, result.max_turns_exhausted stays False and _terminal_status (L475: if self.result.max_turns_exhausted: return FinalStatus.MAX_TURNS_EXHAUSTED) falls through to FAILURE. I reproduced this with the PR's own _SessionAgent fixture and a failing run_command: false criterion. exhaust_on_call=len(TURNS) gives FAILURE mte=False stop=script_exhausted. exhaust_on_call=1 gives MAX_TURNS_EXHAUSTED mte=True stop=agent_max_turns. The same agent behaviour therefore lands in different FinalStatus buckets depending only on which turn it happened on. A one-turn script also diverges from the equivalent single-shot task, which records the fact before grading (L2210-2211). Under execute, the fact is never persisted, so a later evaluate cannot recover it. This is the scoring-correctness class: a status classifier that puts a value in the wrong bucket. The MAX_TURNS final turn of an LLM dialog had the same ordering before this PR, but the PR puts every scripted dialog on this path. Fix: record self.result.max_turns_exhausted = True right after self.result.iterations.append(turn_record) (a fact about the run, as on the single-shot path), and keep only the AGENT_MAX_TURNS break below the stop decision. Add a test that exhausts max_turns on the LAST scripted turn (test_scripted_dialog.py:316 covers only exhaust_on_call=1).
2. [Axis 8] Dataset ${row.*} placeholders not substituted into simulation.turns (src/coder_eval/models/tasks.py:740) — check_scripted_turns_replace_the_prompt (L740-753) forbids initial_prompt beside simulation.turns ("simulation.turns supplies every user turn, the first included: remove 'initial_prompt'"). But expand_dataset (orchestration/task_loader.py) substitutes rows only in data["initial_prompt"] and data["success_criteria"]. It never touches simulation.turns. So for a scripted task with a dataset: block, the only path that injects row data into the prompt is closed, and any ${row.x} in turns reaches the agent verbatim. I reproduced this: a dataset task with turns: ["Question: ${row.q}", "Thanks"] expands to ds-scripted/r1 ['Question: ${row.q}', 'Thanks'] r1.txt and ds-scripted/r2 ['Question: ${row.q}', 'Thanks'] r2.txt. The criteria ARE row-substituted, so N rows grade row-specific expectations against one identical, unrendered prompt, and nothing fails or warns. Fix: substitute ${row.*} in every simulation.turns entry in expand_dataset (and document it in DATASETS.md § what is substituted), or reject dataset: + simulation.turns at validation until that is supported. Add a test for each choice. Static-check candidate: a load-time check that no ${row. survives in any string leaf after expansion.

Non-blocking, but please consider before merge

1. [Axis 1] PR raises complexity of the hottest dialog driver: _simulation_dialog_loop F(41) to F(44), _evaluation_loop D(23) to D(24), partly through an optional user parameter with a fallback built inside the loop (src/coder_eval/orchestrator.py:2440) — I ran radon cc -s on the PR worktree and on origin/main. Orchestrator._simulation_dialog_loop goes from F (41) to F (44) and Orchestrator._evaluation_loop goes from D (23) to D (24), in the hot orchestrator module. Three new branches cause the rise. The first is the optional injected user, which is resolved inside the loop (lines 2471-2481): simulator = (\n user\n if user is not None\n else UserSimulator(config=sim_config, ...)). The other two are the self.grade and guards at 2596 (if self.grade and not criteria_checked_this_turn:) and 2656 (end_of_dialog_needed = self.grade and (). The dispatch at 2168-2175 already branches on if simulation.turns is None:, so it knows which user it needs. Build the DialogUser at the dispatch, for example user = ScriptedUser(turns) if scripted else UserSimulator(...), and make user a required parameter of _simulation_dialog_loop. This removes the None-default parameter, the fallback construction and one decision point. It also stops the 'scripted or LLM' choice from being split between two methods. Consider folding the two self.grade guards into the existing _run_dialog_criteria_check seam, so the loop body does not grow again.
2. [Axis 1] Scripted vs simulated user selected via flat model plus hand-maintained _FIXED_WHEN_SCRIPTED list (fails open), not a discriminated user source (src/coder_eval/models/tasks.py:55) — _FIXED_WHEN_SCRIPTED = ("persona", "goal", "constraints", "model", "stop_token", "stop_on_criteria_pass", "check_criteria") (lines 55-63) is a magic-string list. _validate_user_source loops over it at lines 301-309: if getattr(self, name) != fields[name].get_default(call_default_factory=True). Every future simulator-only knob added to SimulationConfig must also be added to this tuple by hand. If someone forgets, turns silently accepts a knob that does nothing, which is exactly the problem the validator is meant to prevent. Nothing (lint or test) ties the tuple to the model's fields. The same design also weakens persona/goal to str | None for every simulated task, and adds an is_scripted property and a by-value default comparison to work around model_fields_set. Model the user source as a discriminated pair of sub-models instead, for example a scripted spec {turns} and a simulated spec {persona, goal, constraints, model, stop_token, ...}, each with extra="forbid". Pydantic then rejects contradictory keys structurally, persona/goal stay required on the simulated branch, and the tuple, the comparison loop and the None-loosening can be deleted. The CLAUDE.md pattern 'Discriminated unions for criteria types and template sources' is the precedent.
3. [Axis 3] The ungraded-dialog backstop that this PR narrowed has no test on either side (src/coder_eval/orchestrator.py:2310) — The PR adds if self.grade and not criteria_checked_this_turn: (line 2596) and end_of_dialog_needed = self.grade and ( (line 2656). Before the PR, any ungraded dialog that reached a criteria check hit the backstop at 2310-2315 (if not self.grade: raise ValueError("Grading is disabled but the simulation dialog path requires criteria results ...). After the PR, only the per-turn (every_turn/both) check can reach it, and the comment now says it is "reachable ungraded only past the CLI refusal". Line 2311 is in a PR-modified hunk and is uncovered (coverage reports 2311 missing). The only ungraded tests (test_a_script_runs_in_full_ungraded_and_finalizes_not_graded, test_a_budget_breach_ends_an_ungraded_script_with_its_own_status in tests/test_scripted_dialog.py) use scripted dialogs, where the validator forces check_criteria='end_of_dialog'. No test calls Orchestrator(grade=False) on an LLM-simulated dialog. Also, an LLM-simulated end_of_dialog dialog under grade=False used to raise and now finishes NOT_GRADED, and no test asserts that behavior change. Add two tests, with UserSimulator stubbed: (a) an LLM-simulated task with check_criteria='every_turn' and grade=False raises the ValueError, which trips the gate; (b) an LLM-simulated task with end_of_dialog and grade=False finalizes NOT_GRADED with empty success_criteria_results. These tests keep a later change from removing the backstop because it "looks unreachable".
4. [Axis 5] The 'one source of truth for the first prompt' invariant is enforced only when TaskDefinition is built. The variant prompt layer bypasses it, so a variant initial_prompt is silently ignored and the recorded config can no longer be reloaded (src/coder_eval/models/tasks.py:740) — The PR moves the first prompt into a second place (simulation.turns[0]). It guards this only with a TaskDefinition after-validator: def check_scripted_turns_replace_the_prompt(self) -> Self: ... and simulation.is_scripted and (self.initial_prompt is not None or self.initial_prompt_file is not None). The A/B prompt layer (orchestration/experiment.py::_apply_prompt_overrides) changes task.initial_prompt in place after that validation, and nothing validates again (TaskDefinition has no validate_assignment, and apply_overrides validates only the agent/run_limits/sandbox roots). I verified this with a probe. A variant initial_prompt: "VARIANT PROMPT" on a scripted task resolves without error to initial_prompt='VARIANT PROMPT' with turns unchanged. The orchestrator then ignores it (orchestrator.py:2173-2174: return await self._simulation_dialog_loop(simulation.turns[0], sandbox_dir, user=ScriptedUser(simulation.turns))), so the A/B lever does nothing and no message tells the user. Also, TaskDefinition.model_validate(task.model_dump()) on that resolved task now raises simulation.turns supplies every user turn ... remove 'initial_prompt', so the recorded config of the run does not reload for evaluate <run_dir> or resume. Experiment-level defaults.prompt_mutations cause the scripted task to fail resolution with the misleading message initial_prompt must be resolved before applying mutations. The notes added by the PR claim 'the task carries one source of truth for its first prompt', but the merge pipeline breaks that claim. Fix: teach the prompt-override layer about scripted tasks. Either reject a variant initial_prompt / prompt_mutations on a scripted task with a clear message, or apply mutations to simulation.turns on purpose. Also validate the whole TaskDefinition again after _apply_prompt_overrides, so a TaskDefinition invariant cannot be broken by a later layer. Add a resolver test for a variant prompt override on a scripted task.
5. [Axis 7] simulation.max_turns is a redundant, must-set second source of truth for scripted dialog length (src/coder_eval/models/tasks.py:310) — The validator forces max_turns >= len(turns): if len(self.turns) > self.max_turns: raise ValueError(... "raise max_turns to at least {len(self.turns)}.") (tasks.py:310-314). But evaluate_stop checks if config.turns is not None and turns_completed >= len(config.turns): return ... SCRIPT_EXHAUSTED (termination.py:72-73) BEFORE turns_completed >= config.max_turns. Because len(turns) <= max_turns is guaranteed, the script always ends first, so MAX_TURNS can never fire for a scripted dialog. The result is a knob with no effect that a user must still edit: any script longer than the default 8 turns is rejected until the user raises max_turns. Both docs examples carry the same note (max_turns: 3 # Must be at least len(turns). DIALOG_MODE.md:106; max_turns: 8 # Must be at least len(turns). TASK_DEFINITION_GUIDE.md:1809). The progress log also shows a wrong denominator: logger.info("Simulation turn %s/%s", turns_completed, sim_config.max_turns) (orchestrator.py:2544) prints e.g. 2/8 for a 3-turn script. Fix: when turns is set, derive the cap from len(turns). Either add max_turns to _FIXED_WHEN_SCRIPTED and ignore it, or make the effective cap len(turns) and drop the length check. Then remove the 'Must be at least len(turns)' requirement from the field description and both docs.
6. [Axis 7] Scripted tasks cannot share an experiment whose defaults set a simulator knob or prompt_mutations; such tasks are quietly moved to skipped (src/coder_eval/models/tasks.py:55) — _FIXED_WHEN_SCRIPTED = ("persona", "goal", "constraints", "model", "stop_token", "stop_on_criteria_pass", "check_criteria") is checked in _validate_user_source (tasks.py:302-308) against the MERGED SimulationConfig that _resolve_simulation builds from all four layers. The Dialog Mode docs recommend pinning simulation.model for A/B runs. If an experiment does that in defaults.simulation (or sets check_criteria: every_turn for its simulated tasks), every scripted task in the same suite fails. I reproduced it: defaults: {simulation: {check_criteria: every_turn}} gives ValidationError: simulation.turns scripts the user, so these must keep their defaults: ['check_criteria']. resolve_all_tasks catches this ValueError and moves the task to skipped, so a mixed suite gets smaller with only a log warning. The same happens with an experiment-level prompt_mutations: a scripted task (initial_prompt None) raises the misleading initial_prompt must be resolved before applying mutations and is also skipped. Fix: run the 'must keep defaults' rule against the task's own layer only (the authored simulation block), not the inherited experiment layers. Or let a scripted config ignore inherited simulator-only knobs. Also give the prompt_mutations path a clear error, or apply the mutations to turns[0].
7. [Axis 8] The scripted-user validator compares simulator fields to the current code defaults by value, so a later default change makes old recorded scripted runs fail to load (src/coder_eval/models/tasks.py:306) — _validate_user_source rejects a scripted config when getattr(self, name) != fields[name].get_default(call_default_factory=True) (L306) for every name in _FIXED_WHEN_SCRIPTED. That set includes "model" (L59), whose default is DEFAULT_SIMULATOR_MODEL = DEFAULT_JUDGE_MODEL = "anthropic.claude-sonnet-4-6", and "stop_token". The orchestrator records self.recorded_task.model_dump(warnings=False) (orchestrator.py:1075), which bakes today's default model into every scripted run's task_config.resolved. When a release changes DEFAULT_JUDGE_MODEL, regrade.py:102 (TaskDefinition.model_validate(record.resolved)) raises for every scripted run recorded earlier. evaluate <run_dir> / run --resume then takes _fall_back_to_source, which warns "Variant overrides, -D flags and dataset expansion from the original run are NOT reapplied, so this grade may not match what ran". That can grade a config other than the one that ran. The same check also breaks experiment resolution in a mixed suite where defaults.simulation or a variant pins simulation.model, which is the A/B practice the model field description recommends ("hold it fixed to keep the dialog partner constant across variants"). Each scripted task then fails with "these must keep their defaults: ['model']". Fix: make the simulator-only fields None-default (resolved to the constant at use time) and reject only non-None values when turns is set. Alternatively, ignore (and drop) model/stop_token for a scripted user instead of failing on them.

Nits

1. [Axis 1] Dialog loop docstring and local names still say 'LLM user simulator' after the loop is generalized to any DialogUser (src/coder_eval/orchestrator.py:2443) — The docstring at line 2443 still begins """Run the task as a multi-turn dialog driven by an LLM user simulator., and the local variable that can now hold a ScriptedUser is still called simulator (line 2471 simulator = (), as are the simulator: DialogUser parameters of _solicit_user_message (2354) and _acquire_opener (2385). Rename them to user / 'dialog user' and change the summary line to 'driven by a DialogUser (LLM simulator or scripted turns)'. Readers can then see that the scripted path goes through these helpers.
2. [Axis 1] Test helper flag verbatim is an inverted alias for working_directory_prefix (tests/test_scripted_dialog.py:90) — def _task(turns: list[str] = TURNS, *, verbatim: bool = True, **overrides: Any) maps verbatim=True to kwargs["working_directory_prefix"] = False (line 100), and the docstring has to explain that verbatim=False leaves the prefix at its default. Readers must translate the negation at every call site (_task(verbatim=False)). Name the parameter after the model field, for example working_directory_prefix: bool = False, and pass it straight through.
3. [Axis 2] persona/goal loosened to str | None; the simulated-user invariant is now validator-only and UserSimulator interpolates the optional values unnarrowed (src/coder_eval/models/tasks.py:199) — The PR changes persona: str = Field( to persona: str | None = Field(default=None, ...) (line 199), and does the same for goal: str | None (line 206). Only _validate_user_source ("if self.persona is None or self.goal is None: raise ...") enforces that a non-scripted config has both. The consumer _extract_system_prompt in simulation/user_simulator.py:95/98 still interpolates {config.persona} / {config.goal} directly. Pyright accepts str | None in an f-string, so if a scripted config ever reached UserSimulator, the type checker would stay silent and the simulator prompt would contain the literal text "None". Fix: narrow the values once at the UserSimulator boundary (if config.persona is None or config.goal is None: raise ValueError(...) in __init__, then pass str down). Alternatively, give the simulated-user path a typed accessor that returns tuple[str, str], so the invariant is visible to pyright and does not depend only on a model validator. Today the orchestrator only builds UserSimulator when turns is None, so the current code is not wrong.
4. [Axis 3] Duplicate evaluate_stop test; the case where a script is shorter than max_turns is never exercised (tests/test_simulation_termination.py:117) — test_script_exhausted_is_reported_instead_of_max_turns (lines 117-120) makes exactly the same call as test_script_exhausted_after_the_last_turn (lines 112-115): evaluate_stop(config=_scripted_cfg(), turns_completed=3, total_tokens_used=0, criteria_all_passed=False), and it asserts a subset of what the earlier test asserts. Both use _scripted_cfg, which pins "max_turns": 3 next to three turns (line 100). The end-to-end fixture in tests/test_scripted_dialog.py:102 also pins max_turns=len(turns). So no test at any level runs the shape the validator allows and the docs describe ("max_turns must be at least len(turns)"): a script shorter than max_turns, including the default max_turns of 8. In that shape SCRIPT_EXHAUSTED is the only rule that stops the dialog. If it fails to fire, ScriptedUser.next_user_message raises IndexError, _solicit_user_message's except Exception swallows it, and the run is silently mis-bucketed as stop_reason='error' with simulator_failures=1. Replace the duplicate with a case like _scripted_cfg(max_turns=8) at turns_completed=3 (expect SCRIPT_EXHAUSTED), and add one orchestrator test that leaves max_turns at its default.
5. [Axis 3] prompt_mutations and initial_prompt_file on a scripted task are untested and give a misleading error (tests/test_simulation_trials.py:128) — TestScriptedTaskUnderAVariantPrompt covers only ExperimentVariant(variant_id="rewrite", initial_prompt="A different opener.") (line 156). ExperimentVariant and ExperimentDefaults also take prompt_mutations, and variants take initial_prompt_file. In src/coder_eval/orchestration/experiment.py, a scripted task (initial_prompt None) with any default or variant mutation reaches raise ValueError(f"initial_prompt must be resolved before applying mutations (task '{task.task_id}')"). This internal-sounding message does not mention simulation.turns, and an experiment-wide defaults.prompt_mutations breaks every scripted task in the run with it. Add parametrized cases for prompt_mutations (variant and defaults) and initial_prompt_file, and assert an error that names simulation.turns (or assert a deliberate decision to apply mutations to the turns).
6. [Axis 3] New layered fields have no experiment-defaults or -D override coverage (tests/test_simulation_trials.py:85) — Review criterion 10 requires new layered fields to have coverage across the merge layers plus a matching -D path. simulation.turns (MergeField strategy="replace") is tested only at the variant layer (test_variant_turns_replace_the_task_script_wholesale, line 85). working_directory_prefix (new TaskDefinition field) is tested only by direct construction (tests/test_simulation_config.py:262-266). There is no case for experiment defaults.simulation.turns or for -D simulation.turns=[...] / -D working_directory_prefix=false. There is also no cross-layer case where a variant adds turns to a task that already has persona/goal. Under the merge, persona and goal survive next to turns, and _validate_user_source rejects the result, so a variant cannot switch a simulated task to a scripted one. Pin that contract with a test so it is deliberate.
7. [Axis 5] LLM-simulated-dialog predicate duplicated in several ad hoc forms; is_scripted used inconsistently with raw turns-is-None checks (src/coder_eval/cli/run_command.py:994) — The new mode distinction has no single owner. Each consumer writes it again: run_command.py:994 if rt.task.simulation is not None and rt.task.simulation.enabled and not rt.task.simulation.is_scripted; isolation/egress.py:130 if task.simulation is not None and task.simulation.enabled and not task.simulation.is_scripted:; orchestrator.py:2168 if simulation.turns is None: (does not use is_scripted); models/tasks.py:744-746 simulation.enabled and simulation.is_scripted. The simulation.enabled consumers that were not updated have already drifted. models/tasks.py:793 (if self.simulation is not None and self.simulation.enabled and self.sandbox.docker.grading_widens_egress:) still refuses a grading network with the reason 'its criteria run between dialog turns'. A scripted dialog must keep check_criteria at its end_of_dialog default, so its only criteria check runs after the last turn, and the refusal is unnecessary. Fix: add one or two predicates on TaskDefinition/SimulationConfig (for example uses_simulator and reads_criteria_mid_dialog) and send every consumer through them. That includes the grading-network validator and harbor/packager.py:178, whose error message also says 'reads coder-eval's own criteria results mid-dialog'.
8. [Axis 5] DialogUser is a closed union, not a Protocol; LLM user still built inside the loop (src/coder_eval/simulation/__init__.py:27) — DialogUser = UserSimulator | ScriptedUser is a closed union, not a Protocol (start, stop, next_user_message, model). Adding a third user source means editing this alias and every annotation that uses it. Composition is asymmetric. _evaluation_loop builds ScriptedUser, but _simulation_dialog_loop still builds UserSimulator itself unless user is injected (simulator = (user if user is not None else UserSimulator(...))). Its initial_prompt parameter now means both 'pinned opener' and 'turns[0]'. The script end is known in two places. termination.py:72 reads if config.turns is not None and turns_completed >= len(config.turns): and scripted_user.py:37 indexes self._turns[len(dialog_pairs)], which raises IndexError if the two places ever disagree. Fix: define a DialogUser Protocol, let _evaluation_loop build both user kinds and pass a required user, and let the user source report that it is exhausted (or keep the check only in evaluate_stop and document it as the only owner).
9. [Axis 5] SCRIPT_EXHAUSTED missing from HTML report stop-reason label map (src/coder_eval/simulation/termination.py:16) — The PR adds SCRIPT_EXHAUSTED = "script_exhausted" and mirrors it in SimulationTelemetry.stop_reason. But reports/html.py::_SIMULATION_STOP_REASON_LABELS (lines 1069-1076) has no entry for it, so _SIMULATION_STOP_REASON_LABELS.get(sim.stop_reason, ("neutral", sim.stop_reason)) falls back to the raw value and no badge or label is chosen for it. This is the most common end state of a scripted dialog, so the result is real (run_limit_exceeded has the same gap and predates this PR). Fix: add "script_exhausted": ("neutral", "script completed"). Consider a lint/test that checks that the label map covers every DialogStopReason member.
10. [Axis 6] ScriptedUser's documented IndexError (a first-party cursor invariant) is swallowed by the simulator's blanket except and logged as a simulator failure (src/coder_eval/simulation/scripted_user.py:37) — text = self._turns[len(dialog_pairs)] (line 37) is documented to raise IndexError when asked past the last turn (line 21). Its only caller, _solicit_user_message, wraps it in except Exception: logger.exception(failure_log_msg); return _SolicitedMessage(message=None, ...) (orchestrator.py:2372-2374). That handler is meant for an unreliable LLM subprocess. With it, a broken first-party invariant degrades to stop_reason=ERROR, simulator_failures=1, and the log line "User simulator failed — ending dialog". The end-of-dialog grade then still runs, and the row can score SUCCESS even though scripted turns were skipped. The condition is unreachable today because evaluate_stop's SCRIPT_EXHAUSTED check comes first, so this is only hardening. Recommendation: make a scripted cursor overrun fail loud, for example raise a RuntimeError that the loop does not catch, or let the loop skip the solicit call for ScriptedUser. Also use a failure message that does not call a scripted user a 'simulator'.
11. [Axis 8] The grading-network refusal still treats a scripted dialog as one that grades between turns (src/coder_eval/models/tasks.py:793) — check_grading_network_without_simulation refuses whenever if self.simulation is not None and self.simulation.enabled and self.sandbox.docker.grading_widens_egress:, with the reason "its criteria run between dialog turns". A scripted dialog is pinned to check_criteria="end_of_dialog" by _FIXED_WHEN_SCRIPTED, so its only criteria check runs after the last turn, the same as single-shot. The PR made egress.py (L130) and the execute refusal scripted-aware but left this guard as it was, so one concept now has inconsistent rules. Either exempt simulation.is_scripted here (with a test that the grading network opens only after the last turn), or record in DOCKER_ISOLATION.md why scripted dialogs stay excluded.

What's Missing

Parallel paths:

• 🟡 The dataset fan-out was not updated. orchestration/task_loader.py::expand_dataset substitutes ${row.*} only into initial_prompt and success_criteria. The PR makes simulation.turns the only prompt source for a scripted task, so every row sends the same unrendered turns while its criteria are row-specific. Fix: substitute into turns, or reject dataset: + turns. (trigger: src/coder_eval/models/tasks.py) (restates: Axis 8: Dataset ${row.*} placeholders not substituted into simulation.turns)
• 🟡 The A/B prompt layer was not updated. orchestration/experiment.py::_apply_prompt_overrides does not know about scripted tasks. A variant initial_prompt is set but silently ignored (the orchestrator sends turns[0]), and the recorded config then fails to reload. Any prompt_mutations raises the misleading 'initial_prompt must be resolved before applying mutations'. (trigger: src/coder_eval/models/tasks.py) (restates: Axis 5: The 'one source of truth for the first prompt' invariant is enforced only when TaskDefinition is built)
• 🟡 The judge prompt was not updated. evaluation/judge_context.py::DIALOG_HEADER, used by both llm_judge and agent_judge, still says 'in simulation mode the USER side is generated by an LLM simulator and may invent premises — treat any claim made only by the simulated user as possibly fabricated'. On a scripted dialog the user turns are the authored benchmark questions, so the grader is told to distrust ground truth. The PR lists this under 'Known limits' but does not fix it. Fix: choose the header by simulation.is_scripted. (trigger: src/coder_eval/simulation/scripted_user.py)
• 🔵 Several other consumers of simulation.enabled were not made scripted-aware, although run_command.py and egress.py were. The grading-network refusal (models/tasks.py:793, 'criteria run between dialog turns') and the stop_early refusal (orchestration/early_stop.py:124) still refuse a scripted dialog, although it reads criteria only after the last turn. The harbor/packager.py:178 message ('reads criteria mid-dialog') and the task_loader.py:189-193 message ('the simulator generates the opener') are wrong for scripted dialogs. (trigger: src/coder_eval/isolation/egress.py) (restates: Axis 5: LLM-simulated-dialog predicate duplicated in several ad hoc forms)
• 🔵 Orchestrator._resolve_routes still resolves simulator_route for a scripted task, and orchestrator.py:1755-1758 records environment_info.simulator_routing / simulator_model for it. So a scripted run names a simulator model that never ran, while SimulationTelemetry.simulator_model is null and egress.py drops the simulator route. Fix: skip recording these keys when simulation.is_scripted. (trigger: src/coder_eval/orchestrator.py)

Tests:

• 🟠 No test exhausts the agent's run_limits.max_turns on the LAST scripted turn. test_an_agent_that_runs_out_of_its_own_turns_ends_the_script covers only exhaust_on_call=1, so the SCRIPT_EXHAUSTED break before max_turns_exhausted is recorded is untested. That ordering puts the row in FAILURE instead of MAX_TURNS_EXHAUSTED. (trigger: tests/test_scripted_dialog.py) (restates: Axis 8: Scripted dialog loses agent max_turns_exhausted on final turn)
• 🟡 The new self.grade and guards in _simulation_dialog_loop have no test for an LLM-simulated dialog. Two cases are missing: grade=False with every_turn hits the ValueError backstop, and grade=False with end_of_dialog now finalizes NOT_GRADED where before it raised. Only scripted dialogs run with grade=False in tests. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 3: The ungraded-dialog backstop that this PR narrowed has no test on either side)
• 🟡 No test uses a script shorter than max_turns, including the default of 8. Every fixture pins max_turns=len(turns), so the case where only SCRIPT_EXHAUSTED can stop the dialog is never run. A regression would surface as an IndexError, which is swallowed and reported as stop_reason='error'. (trigger: tests/test_simulation_termination.py) (restates: Axis 3: Duplicate evaluate_stop test; the case where a script is shorter than max_turns is never exercised)
• 🔵 The new layered fields have no merge-layer tests. Nothing covers experiment defaults.simulation.turns, -D simulation.turns=[...] / -D working_directory_prefix=false, or a mixed suite whose defaults.simulation sets a simulator knob (model, check_criteria). That last case currently moves every scripted task to skipped. (trigger: tests/test_simulation_trials.py) (restates: Axis 3: New layered fields have no experiment-defaults or -D override coverage)
• 🔵 Before this PR a dialog row could never be NOT_GRADED. Now an execute run can leave a scripted dialog row NOT_GRADED, and no test runs run --resume on it. The test should show that resume grades the dialog in place and does not re-run it. There is also no test with one scripted and one LLM-simulated task under one execute that asserts the refusal names only the LLM-simulated task. (trigger: src/coder_eval/cli/run_command.py)

Display & mapping dicts:

• 🔵 reports/html.py::_SIMULATION_STOP_REASON_LABELS has no entry for script_exhausted, the normal end of every scripted dialog, so the badge falls back to the raw value. No test ties the three hand-kept copies together: DialogStopReason, the SimulationTelemetry.stop_reason Literal, and this label map. (trigger: src/coder_eval/simulation/termination.py) (restates: Axis 5: SCRIPT_EXHAUSTED missing from HTML report stop-reason label map)
• 🔵 Reports and logs do not show whether the user was scripted. The HTML Simulation section shows 'Simulator tokens in 0 · out 0' and labels errors 'simulator error' for a scripted dialog. The orchestrator logs 'User simulator failed — ending dialog' for a ScriptedUser. (trigger: src/coder_eval/orchestrator.py) (restates: Axis 6: ScriptedUser's documented IndexError is swallowed by the simulator's blanket except and logged as a simulator failure)

Downstream consumers:

• 🟡 The new SCRIPT_EXHAUSTED exit runs before result.max_turns_exhausted is set, so _terminal_status (and every MAX_TURNS_EXHAUSTED count and pass-rate built on it) classifies a final-turn agent cap as FAILURE. The progress log (orchestrator.py:2544) also still divides by sim_config.max_turns, not len(turns). (trigger: src/coder_eval/simulation/termination.py) (restates: Axis 8: Scripted dialog loses agent max_turns_exhausted on final turn)

Nightly pipeline:

• 🟡 The PR does not say what a routine release does to recorded scripted runs. _validate_user_source compares model/stop_token by value with today's defaults. The next DEFAULT_JUDGE_MODEL bump will make every earlier scripted run's task_config.resolved fail validation, and nightly evaluate <run_dir> / run --resume then falls back to re-reading the source task YAML. (trigger: src/coder_eval/models/tasks.py) (restates: Axis 8: The scripted-user validator compares simulator fields to the current code defaults by value)
• 🔵 The cross-repo contract changes, but the PR body and docs/REPORT_SCHEMA.md do not say so. task.json simulation.stop_reason gains script_exhausted, simulator_model can be null, and the recorded task_config.resolved gains working_directory_prefix. An older pinned coder-eval reading these runs (for example the coder-eval-uipath pipeline) rejects them, because the stop_reason Literal is closed and the models use extra="forbid". Existing nightly tasks keep their behaviour, because working_directory_prefix defaults to true. State this in the PR. (trigger: src/coder_eval/models/results.py)

Harness & Lint Improvements

Static checks (lint / type):

• [ce-lint] Widen CE022 (tests/lint/rules/ce022_dialog_loop_statement_cap.py) so it also caps the radon cyclomatic complexity of Orchestrator._simulation_dialog_loop, and add _evaluation_loop as a second target. radon is already a runtime dependency. Set the caps to main's values (41 and 23), which are below the PR's values. The function keeps # noqa: PLR0915, and the statement cap of 128 lets a branch-only change through. Ruff's PLR0912 is the only other bound, so a change that adds branches but stays under the statement count passes today. The new message must say: build the DialogUser at the dispatch, do not add a None-default fallback inside the loop. Prevents: Finding 1 (orchestrator.py:2440): the PR raised _simulation_dialog_loop from F(41) to F(44) and _evaluation_loop from D(23) to D(24) and still passed CE022. The cause was the optional user fallback built inside the loop plus the two self.grade and guards.
• [ce-lint] New CE069, a closed-set mirror completeness rule in tests/lint/rules/ce069_closed_set_mirror_complete.py, wired in tests/lint/runner.py. CE067 is retired and must not be reused. CE068 is taken. Collect every StrEnum in src/coder_eval. (a) In reports/, a module-level dict literal whose str-constant keys include 2 or more values of one StrEnum must cover every value of that enum, or the code must be changed to a match + assert_never dispatch like early_stop_gate_note in reports/markdown.py. (b) A Literal[...] annotation in models/ whose string args overlap 3 or more values of one StrEnum must equal that enum's value set. Delete-before-guard: move DialogStopReason into models/ and annotate SimulationTelemetry.stop_reason with the enum, so check (b) has nothing left to mirror. Prevents: Finding 13 (termination.py:16 / reports/html.py:1069): script_exhausted is missing from _SIMULATION_STOP_REASON_LABELS, and the existing run_limit_exceeded gap is the same miss. It also catches the next hand-copy of the stop_reason Literal in models/results.py:432 drifting from DialogStopReason.
• [ce-lint] Pyright tightening: set reportMatchNotExhaustive = "error" in [tool.pyright]. The standard mode leaves it off, so the assert_never idiom is today only a runtime guard. With this setting, a match over DialogStopReason / FinalStatus / EarlyStopReason that is missing a member fails make typecheck. This is the type-level partner of CE069's match option. Prevents: Finding 13 (the missing SCRIPT_EXHAUSTED label), after the html label map becomes a match. It also prevents any later stop-reason / status renderer from falling through for a new enum member.
• [ce-lint] New CE070, a mode-field partition check for SimulationConfig. Add it as a @pytest.mark.lint class in tests/test_custom_lint.py next to CE031, because it reflects over a model and does not walk one AST. Every name in SimulationConfig.model_fields must be in exactly one of _FIXED_WHEN_SCRIPTED (simulator-only) or a new explicit _APPLIES_TO_SCRIPT allow-list (enabled, turns, max_turns, max_total_tokens, n_trials). The check fails when a field is in neither list, in both lists, or names a field that does not exist. The test must also stop copying the 7 names by hand at tests/test_simulation_config.py:198, and import the tuple instead. Delete-before-guard: the preferred fix is a discriminated scripted/simulated sub-model with extra="forbid", which deletes the tuple and this rule together. Prevents: Finding 2 (tasks.py:55): a fail-open deny-list where a forgotten simulator-only field is accepted beside turns and does nothing. It also prevents Finding 15 (tasks.py:310), because a partition forces an explicit decision on whether max_turns applies to a script, and it would have exposed max_turns as a redundant second source for script length.
• [ce-lint] New CE071, a single owner for the dialog-mode predicate, in tests/lint/rules/ce071_dialog_mode_predicate.py. Forbid the attribute chain <x>.simulation.enabled and <SimulationConfig>.turns is None / is not None outside src/coder_eval/models/tasks.py. Consumers go through named properties on TaskDefinition, for example runs_dialog, uses_llm_simulator and reads_criteria_mid_dialog. Sites to move (on pr-246): run_command.py:994, isolation/egress.py:130, orchestrator.py:2167-2168, harbor/packager.py:178, orchestration/early_stop.py:124, orchestration/task_loader.py:189. Prevents: Finding 11 (run_command.py:994 / egress.py:130 / orchestrator.py:2168): the scripted-vs-simulated predicate is written again in each consumer. Finding 20 (tasks.py:793): the grading-network refusal and the harbor packager still treat a scripted dialog as one that reads criteria mid-dialog, because they were not routed through a shared predicate.
• [ce-lint] New CE072, one writer for turn-derived result facts, in tests/lint/rules/ce072_turn_facts_single_writer.py. Assignment to self.result.max_turns_exhausted (and to any EvaluationResult field copied from a TurnRecord) is allowed only inside one Orchestrator._record_turn_facts(turn_record) helper, plus the resume copy at orchestrator.py:708, which is exempt. Delete-before-guard: the helper is called right after self.result.iterations.append(turn_record) on the single-shot path and the dialog path, so no stop-decision break can come between the turn and the recording of its facts. The rule finds the duplicated writer sites (orchestrator.py:2210 and 2618) statically. The ordering inside the loop needs the runtime test in the harness bucket. Prevents: Finding 17 (orchestrator.py:2608, high): the SCRIPT_EXHAUSTED break runs before self.result.max_turns_exhausted = True, so a scripted dialog whose agent exhausts max_turns on the last turn is classified FAILURE and not MAX_TURNS_EXHAUSTED.
• [ce-lint] New CE073: in src/coder_eval/models/, no validator may compare a field to its current code default by value. This forbids .get_default( calls and == / != DEFAULT_* comparisons inside @model_validator / @field_validator bodies. Every recorded task_config.resolved goes through these validators again on evaluate / --resume, so a check that depends on a default turns a later default change into a reload failure. Express 'not authored' as a None default that is resolved at use time. Only one site exists today (tasks.py:306), so the rule is cheap. The shape is greppable, and the fix removes the only occurrence. Prevents: Finding 19 (tasks.py:306): when DEFAULT_JUDGE_MODEL changes, every earlier scripted run fails TaskDefinition.model_validate(record.resolved) and regrade falls back to source. Also Finding 16 (tasks.py:55): an inherited defaults.simulation.model breaks every scripted task in a mixed suite.
• [ci-gate] Add a diff-coverage gate to make verify / CI, for example diff-cover coverage.xml --compare-branch=origin/main --fail-under=90, limited to changed lines under src/coder_eval/. The 80% global threshold cannot see one uncovered branch in a PR-modified hunk. This gate makes 'new branch shipped untested' a mechanical failure and not something a reviewer has to remember. Prevents: Finding 6 (orchestrator.py:2310-2311): the ungraded-dialog backstop that the PR narrowed is uncovered in a modified hunk. It partly prevents Finding 7 (no test runs a script shorter than max_turns) and Finding 8 (the prompt_mutations/initial_prompt_file paths on a scripted task).

Harness improvements (not statically reachable):

• Add a stop-path x exhaustion-turn parity test matrix in tests/test_scripted_dialog.py. For each dialog end (SCRIPT_EXHAUSTED, MAX_TURNS, STOP_TOKEN, CRITERIA_PASSED), make the _SessionAgent fixture exhaust its agent max_turns on the first, a middle and the last turn, and assert the same FinalStatus and result.max_turns_exhausted. Also assert that a one-turn script and the equivalent single-shot task produce the same FinalStatus and result facts, under run and under execute followed by evaluate. Why not static: The defect is the run-time order of a stop-decision break and a fact assignment inside one loop iteration. CE072 can force a single writer, but only running the loop shows which statement runs first on which exit. Prevents: Finding 17 (orchestrator.py:2608, high). It also gives the orchestrator test with a default max_turns (script shorter than the cap) that Finding 7 asks for.
• Add a guard after dataset expansion. When expand_dataset has run, walk every string leaf of each expanded task's model_dump() and raise if a ${row. placeholder survives. Today only initial_prompt and success_criteria are substituted, so this guard makes a field outside that list (now simulation.turns, tomorrow any other prompt-bearing field) fail loud and not reach the agent verbatim. Pair it with a DATASETS.md 'what is substituted' entry for simulation.turns, or with a validation error for dataset: + simulation.turns. Why not static: The placeholder is in user-authored YAML data, not in source code. Only the loaded and expanded task shows whether a row placeholder is left in a field that substitution does not cover. Prevents: Finding 18 (tasks.py:740 / task_loader.py:436-440, high): N dataset rows grade row-specific criteria against one unrendered ${row.q} prompt, and nothing warns.
• Add a round-trip invariant for the resolved config. At the end of resolve_all_tasks, after _apply_prompt_overrides, CLI overrides and dataset expansion, require TaskDefinition.model_validate(task.model_dump()) to succeed, and raise a clear error that names the layer. This makes sure that a TaskDefinition invariant broken by a later in-place layer (TaskDefinition has no validate_assignment) fails at resolve time, and that a run is never recorded with a config that evaluate / --resume cannot reload. Back it with a parametrized resolver test on a scripted fixture over: variant initial_prompt, variant/defaults prompt_mutations, variant initial_prompt_file, defaults.simulation.model, defaults.simulation.check_criteria, -D simulation.turns=[...], -D working_directory_prefix=false, and a variant that adds turns to a persona/goal task. Why not static: The invariant breaks only when one merge layer's data meets a later layer's in-place mutation. There are many legitimate task.<field> = assignments in orchestration/, so an AST rule cannot separate safe ones from unsafe ones without the merged values. Prevents: Finding 10 (tasks.py:740): a variant initial_prompt on a scripted task is silently ignored and the recorded config cannot be reloaded. Finding 8 (misleading initial_prompt must be resolved before applying mutations). Finding 9 (no defaults / -D coverage for the new layered fields).
• Add a forward-compatibility test for recorded configs. Load a checked-in task_config.resolved fixture for a scripted run, monkeypatch each simulator code default (DEFAULT_JUDGE_MODEL / DEFAULT_SIMULATOR_MODEL, stop_token), and assert that orchestration/regrade.py reloads it without taking _fall_back_to_source. Why not static: The failure is temporal: the recorded config was valid when it was written and becomes invalid only after a later default change. Seeing it needs the persisted artifact plus a changed default at run time. Prevents: Finding 19 (tasks.py:306): old scripted runs silently regrade from source after a default bump.
• Make a mixed-suite resolution regression fail loud. Add an experiment fixture that holds a single-shot task, an LLM-simulated task and a scripted task under defaults: that pin simulation.model and set prompt_mutations, and assert that no task goes to skipped. Also consider counting resolution-skipped tasks in run.json / the summary, so a suite that gets smaller is visible to the external eval-runner consumer and not only in a log warning. Why not static: The skip happens only when experiment-level defaults are merged with a specific task mix at resolve time. The code that catches the ValueError is correct by itself. The defect is the data combination. Prevents: Finding 16 (tasks.py:55): scripted tasks are quietly moved to skipped when experiment defaults set a simulator knob or prompt_mutations. Finding 15 (tasks.py:310): a script longer than 8 turns is rejected until max_turns is raised.
• Add a test that trips each grade-disabled backstop. For every if not self.grade: raise in orchestrator.py, require a test that reaches it: an LLM-simulated check_criteria='every_turn' dialog with grade=False raises, and an LLM-simulated end_of_dialog dialog with grade=False finalizes NOT_GRADED with empty success_criteria_results. In the same file, replace the duplicate test_script_exhausted_is_reported_instead_of_max_turns with a _scripted_cfg(max_turns=8) case, and add a test that a ScriptedUser cursor overrun propagates and is not logged as a 'simulator failure'. Why not static: These are behaviour assertions on paths that are reachable only past the CLI refusal, or only when two owners of 'end of script' disagree. A lint rule can see that a backstop exists, but not that a test exercises it. The diff-coverage gate finds the uncovered line, and these tests pin the contract. Prevents: Finding 6 (orchestrator.py:2310). Finding 7 (test_simulation_termination.py:117). Finding 14 (scripted_user.py:37): the IndexError is swallowed by the blanket simulator except.
• Record the boundary of static analysis for the remaining findings in .claude/harness-candidates.md, so it is a deliberate decision. (a) A str | None value interpolated into an f-string (UserSimulator {config.persona}) is not flagged by any pyright setting, and a CE rule would need type information. The fix is to narrow once at the UserSimulator boundary. (b) Stale 'LLM user simulator' naming, the inverted verbatim test flag, and DialogUser as a closed union and not a Protocol need semantic judgment and stay reviewer-owned. Why not static: Detecting these needs inferred types through f-string formatting, or a judgment about naming and abstraction, which an AST or grep cannot make reliably. Prevents: Finding 5 (tasks.py:199), Finding 3 (orchestrator.py:2443), Finding 4 (test_scripted_dialog.py:90), Finding 12 (simulation/init.py:27). They are recorded as reviewer-owned, not as mechanically prevented.

Top 5 Priority Actions

1. In src/coder_eval/orchestrator.py:2602-2619, set self.result.max_turns_exhausted = True right after the turn record is appended and before the evaluate_stop break, because SCRIPT_EXHAUSTED is now the normal scripted exit; today an agent that hits its max_turns on the last scripted turn is graded FAILURE instead of MAX_TURNS_EXHAUSTED. Add a test with exhaust_on_call=len(TURNS) (tests/test_scripted_dialog.py:316 covers only call 1).
2. Substitute ${row.*} into every simulation.turns entry in expand_dataset (orchestration/task_loader.py:436-440), or reject dataset: together with simulation.turns at validation. Today tasks.py:740 forbids initial_prompt, so every dataset row sends the same literal placeholder text while its criteria are row-specific, and nothing warns.
3. Make _apply_prompt_overrides (orchestration/experiment.py:278-298) handle scripted tasks: reject a variant initial_prompt / prompt_mutations with an error that names simulation.turns, or apply them to the turns on purpose. Then validate the whole TaskDefinition again. Today a variant prompt is silently ignored (orchestrator.py:2173), and the recorded config fails the tasks.py:740 validator when evaluate or --resume reloads it.
4. Replace the by-value default comparison over _FIXED_WHEN_SCRIPTED (src/coder_eval/models/tasks.py:55-63, 306) with None-default simulator-only fields, an allow-list, or a discriminated scripted/simulated sub-model. Today a future DEFAULT_JUDGE_MODEL change breaks regrade of recorded scripted runs (regrade.py:102 falls back to source), defaults.simulation.model or check_criteria quietly moves scripted tasks to skipped, and a new simulator field left off the list fails open.
5. When turns is set, derive the dialog cap from len(turns) and remove the max_turns >= len(turns) check (src/coder_eval/models/tasks.py:310-314), the matching notes in DIALOG_MODE.md:106 and TASK_DEFINITION_GUIDE.md:1809, and the wrong /max_turns denominator in the progress log at orchestrator.py:2544. Add tests for a script shorter than the default max_turns and for the ungraded backstop at orchestrator.py:2310.

---

Stats: 0 🔴 · 2 🟠 · 7 🟡 · 11 🔵 across 8 axes reviewed.

[github-actions]

Claude finished @cosminacho's task in 1m 31s —— View job

---

Code Review for PR #246: feat(simulation): add scripted user turns

Working on review...

Todo List:

• Read .github/code_review.md for review standards
• Read CLAUDE.md for project conventions
• Run git diff origin/main...HEAD to see full changes
• Read full changed files for context
• Perform comprehensive review following guidelines
• Check cross-file consistency
• Identify what's missing
• Format and post final review

[bai-uipath]

Approve, with one fix before merge. Reusing the existing dialog loop with a drop-in scripted user is the right shape.

• Variant prompt overrides are silently ignored on scripted tasks (before merge): the variant initial_prompt is applied by plain attribute assignment, so the new "no prompt beside turns" validator never runs. The run uses turns[0] and drops the variant prompt without a word, and the resolved record then fails validation on reload, so evaluate / run --resume on that run crash in regrade. Reproduced on 31f4c63. Fix: reject a variant initial_prompt on scripted tasks when overrides are applied; prompt_mutations already fails there, but with a confusing "initial_prompt must be resolved" error, so give it the same clear message.
• max_turns is a second source of truth: with the default of 8, a 9-question script fails validation until the author also raises max_turns. Fix: derive it from the script length for scripted dialogs.

Minor: model on the must-stay-default list means an experiment that pins simulation.model suite-wide rejects every scripted task in it; persona / goal are now optional at the type level, with the invariant enforced only by the validator.