feat: Multi-turn eval #1441
Open
chiang-daniel wants to merge 39 commits into
Open
Conversation
Removes the /respond SDK module and its supporting wire types (RespondRequest/Response, SyntheticUserDriverConfig, ConversationTurn, the nested SyntheticUserInfo model). Per-turn synthetic-user invocation moves to OSS at libs/core/kiln_ai/synthetic_user/ in a subsequent commit. Collapses SyntheticUserCase.synthetic_user_info to a single tagged blob string: <persona>...</persona><goal>...</goal><behavior_guidance>...</behavior_guidance> The server treats the blob as opaque; the local player parses it. Adds a typed `code` literal on /generate's 502 response (llm_unavailable | upstream_invalid_output) so callers can discriminate between transient model failures and unparseable model output. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
OSS-side per-turn synthetic-user invocation — the replacement for kiln_server's removed /respond endpoint. Lives in libs/core/kiln_ai/synthetic_user/ so the runner can call the LLM using the user's own provider keys rather than a hosted endpoint. Modules: - models — Pydantic SyntheticUserInfo (parsed form) + SyntheticUserDriverConfig. - parser — tagged-blob ↔ SyntheticUserInfo. Required: <persona>, <goal>; optional: <behavior_guidance>. Unknown tags ignored (forward-compat). - role_swap — flips eval-frame user/assistant labels into LLM-frame labels; raises on system/tool roles (the driver filters those upstream) and on non-string content. - prompt — persona-playing system prompt. No <DONE>/<CANCEL> guidance: drive loop is fixed-length; SU stays engaged across the conversation. - driver — SyntheticUserDriver. Parses the blob once at construction, renders the system prompt once, builds the adapter once. respond() filters visible roles, role-swaps, prepends the system prompt as prior_trace[0], calls adapter.invoke_returning_run_output (in-memory — the SU never persists a TaskRun), returns the raw string. 56 unit tests covering: parser roundtrip / required-tag enforcement / whitespace / unknown-tag forward-compat; role_swap empty/alternating/ preserves-order/raises-on-system-or-tool; prompt structural assertions (persona/goal/conventions present, behavior_guidance only when set, no <DONE>/<CANCEL>); driver happy path, role-swap shape, custom visible_roles, ends-on-assistant invariant, non-string output guard, parse-error on construction, adapter reuse across turns. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Picks up an OpenAPI description on GenerateSyntheticUsersResponse.cases documenting the strict-N batch contract. No shape change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Thin async wrapper around the SDK's /v1/synthetic_user/generate endpoint. The SDK now parses 401/422/500/502 into typed response models, so the wrapper switches on the parsed type rather than reading raw bytes — 502 surfaces its typed `code` literal (llm_unavailable | upstream_invalid_output) directly to callers. No retry loop. /generate is a once-per-batch authoring call; kiln_server's pipeline already retries transient provider failures internally before returning 502, so a 502 reaching us is a genuine per-batch failure that should propagate. Drops the v1 client's SyntheticUserTransientError + backoff machinery. No /respond. Per-turn synthetic-user invocation lives at libs/core/kiln_ai/synthetic_user/ and runs locally with the user's keys. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds an explicit "your entire output is the user's next message, verbatim
and nothing else: no narration, no meta-commentary, no quotes, no labels
like 'User:'" clause to the persona-playing system prompt.
A team running similar SU-driven evals reported the persona-playing
model frequently breaks character — narrating ("I would now ask..."),
self-evaluating, or labeling its output. This clause pins that down at
the prompt boundary so we don't end up reaching for post-processing
band-aids later.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
drive_loop.py:
- drive_case(*, case, target_invoker, su_driver, turns, on_turn) runs the
loop for exactly `turns` iterations — no early termination, no
stop_signal plumbing. Returns DriveCaseResult(chain) with the persisted
TaskRun chain.
- TargetInvoker + TurnHook Protocols. The SU driver does all role
filtering / role swap / invariant checks internally; the drive loop
passes the cumulative trace as-is.
runner.py:
- run_cases_batch is an async generator yielding typed BatchEvents
(BatchStartedEvent / TurnCompletedEvent / CaseCompletedEvent /
CaseFailedEvent / BatchCompletedEvent). No stop_signal/stop_reason
fields — drive loop is fixed-length.
- Constructs a SyntheticUserDriver per case; a malformed
synthetic_user_info blob surfaces as a CaseFailedEvent for that case
alone (other cases continue).
- _make_target_invoker / _build_input_source / _tag_leaf patterns kept
from the prior v1 commits (target persistence + SU attribution
unchanged). input_source now carries the opaque blob on the root run
+ slim {batch_tag, turn_index} on subsequent turns.
- Per-case try/except now WRAPS _tag_leaf too, so a save_to_file failure
surfaces as case_failed instead of silently disappearing into
asyncio.gather(return_exceptions=True). Same try also wraps the
target_invoker construction.
- Case tasks are kicked off before the first BatchStartedEvent yield and
the entire drain loop is inside a try/finally that cancels them on
consumer disconnect — fixes the v1 issue where browser disconnect kept
the request alive for the full duration of every in-flight case.
14 tests cover: input validation, happy-path event stream, leaf tagging,
auto-generated batch_tag, malformed blob → case_failed, target invoke
failure → case_failed, tag-save failure → case_failed, concurrency
semaphore enforcing max-in-flight, root vs slim input_source
attribution, and consumer cancellation propagating to case tasks.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two routes for the multi-turn synthetic-user data-generation pipeline: - POST .../multiturn_sdg/generate_cases (sync JSON) - POST .../multiturn_sdg/run_cases_batch (SSE via CancellableStreamingResponse) Wires connect_multiturn_sdg_api into desktop_server.make_app and registers the Multiturn SDG tag in kiln_server's tags_metadata so the regenerated api_schema.d.ts surfaces the routes in the typed client. Both routes guard task.turn_mode == multiturn before doing any upstream work and route SyntheticUserClient typed errors through to faithful HTTP statuses (401/422/502 preserved, not collapsed). The SSE route threads build_save_context(request) into run_cases_batch and uses an isinstance whitelist on the JSON encoder so future Pydantic types on the wire need explicit review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Rename total_cost -> target_total_cost on CaseCompletedEvent and BatchCompletedEvent. The runner only sees target adapter spend; the SU driver's per-turn cost isn't rolled up here. Old name was misleading in a beta where users pick the SU model. - Thread an optional save_context through run_cases_batch and wrap the leaf-tag save. Adapter writes inside adapter.invoke still bypass — a kiln_ai-side gap shared with the chat SSE pattern, documented in the runner docstring. - Add a re-run idempotency test for _tag_leaf to lock in the spec's "set-union + sort, preserves pre-existing tags" contract. - Drop the dead UNSET/None branch in client._code_or_default; the remaining one-liner has identical behavior. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Rename DEFAULT_TURNS -> MAX_TURNS_DEFAULT to match spec naming. - Name asyncio.create_task instances so debug dumps point at this code. - Pre-assert non-empty seed_prompt in drive_case (assert-loud invariant). - Document invariants on _make_target_invoker (sequential-per-case), _tag_leaf (one-writer-per-leaf), and _close_when_done (final put on cancel path goes into the void). - Drop the unreachable generic fallback in _to_http_exception; tighten the param type to the two real subclasses so the type checker enforces exhaustiveness at the call site. - Log a warning in _format_validation_detail when every item is skipped so a silent SDK shape drift surfaces. - Tests: parameterize turns<1 with negatives, lock in _event_to_payload's unregistered-event guard, and couple the auto-batch_tag test to the public regex instead of the implementation. - Stale "Phase 3" docstring scrub + f-string cosmetic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Root TaskRun's input_source.properties now carries the decomposed SU case context — persona, goal, behavior_guidance (when present), seed_prompt — instead of the opaque tagged blob. Lets dataset readers and eval tooling inspect SU attribution by direct property access rather than re-parsing the XML each time. The blob is losslessly reconstructable from these fields via build_synthetic_user_info if a downstream tool needs the original wire form. Parse happens once per case in _build_input_source on the root turn; the SU driver constructor already validated the blob, so the re-parse here can't surface a new error class. behavior_guidance is omitted when the parser returns None (the DataSource validator rejects empty strings). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SyntheticUserDriver.respond now returns (message, cost) — the per-call cost is read from the in-memory TaskRun's usage.cost (the only place SU spend surfaces, since SU turns aren't persisted as TaskRuns). drive_case accumulates su_total_cost across turns and exposes it on DriveCaseResult. The runner adds it to the leaf's cumulative_usage.cost to produce an honest CaseCompletedEvent.total_cost — renamed from target_total_cost since the field now reports total spend, not just the target adapter's. BatchCompletedEvent.total_cost sums across successful cases the same way. Matters now because the SU model is user-selectable: someone picking Sonnet for higher-quality probes would have had ~half their spend invisible under the old target-only total. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…source Every input to the filter has stronger upstream protection now: seed_prompt is asserted non-empty in drive_case; persona and goal are required-non-empty by parse_synthetic_user_info; behavior_guidance is already conditionally skipped if None; the remaining keys are Pydantic- validated or non-string. The filter was guarding nothing. The DataSource validator stays as the real backstop. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pure relocation + boundary update; behavior unchanged.
run_cases_batch and drive_case now live at
libs/core/kiln_ai/synthetic_user/{runner,drive_loop}.py alongside the
existing SyntheticUserDriver. Same neighborhood as EvalRunner /
RagJobRunner / ExtractorRunner — runners belong in libs/core.
To make libs/core SDK-agnostic, introduce a small
kiln_ai.synthetic_user.SyntheticUserCase Pydantic model (two fields,
field-identical to the kiln_server SDK's case shape). The
multiturn_sdg_api route validates dicts straight into the libs/core type
via Pydantic, so the runner never sees the SDK class. The SDK case is
still used for `/generate_cases` output via `to_dict()` — nothing
about that pro-gated authoring path changes.
Tests move with the code. studio_server keeps only the SDK-wrapper
SyntheticUserClient and the FastAPI route, which is exactly the
established shape for eval_api driving EvalRunner.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds synthetic-user models, parsing and prompt helpers, per-turn and batch runners, desktop multiturn APIs, Copilot multi-turn save support, and frontend builder/review pages. Generated schema, route metadata, and tests are updated to match the new flows. ChangesMulti-turn synthetic data generation
Sequence Diagram(s)Multiturn case generation and batch streaming sequenceDiagram
participant Client
participant multiturn_sdg_api
participant SyntheticUserClient
participant run_cases_batch
Client->>multiturn_sdg_api: POST /generate_cases
multiturn_sdg_api->>SyntheticUserClient: generate(...)
SyntheticUserClient-->>multiturn_sdg_api: cases
multiturn_sdg_api-->>Client: GenerateCasesApiOutput
Client->>multiturn_sdg_api: POST /run_cases_batch
multiturn_sdg_api->>run_cases_batch: cases, target_run_config, su_driver, turns
run_cases_batch-->>multiturn_sdg_api: BatchEvent frames
multiturn_sdg_api-->>Client: SSE stream
Multi-turn save and rollback sequenceDiagram
participant Client
participant create_spec_with_copilot
participant find_multi_turn_chain_leaves
participant tag_multi_turn_chains_for_eval
participant untag_multi_turn_chains_for_eval
participant TaskRun
Client->>create_spec_with_copilot: multi_turn request
create_spec_with_copilot->>find_multi_turn_chain_leaves: batch_tag
create_spec_with_copilot->>tag_multi_turn_chains_for_eval: eval_tag, golden_tag
tag_multi_turn_chains_for_eval->>TaskRun: save_to_file()
alt later step fails
create_spec_with_copilot->>untag_multi_turn_chains_for_eval: tagged_leaves
untag_multi_turn_chains_for_eval->>TaskRun: save_to_file()
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces multi-turn synthetic data generation (SDG) capabilities, adding FastAPI routes, a local synthetic-user driver, client wrappers, and comprehensive unit tests, alongside updates to tracking models. The review feedback highlights several critical issues: multiple model files (chat_session_list_item.py, kiln_base_model.py, task_output.py, task_output_rating.py, and task_run.py) use datetime.datetime.fromisoformat without importing the datetime module, which will cause runtime NameErrors. Additionally, manually overriding the Content-Type header with a hardcoded boundary in the prompt optimization endpoint is fragile and should be removed, and role_swap.py needs to gracefully handle None content in assistant messages to prevent crashes during tool-use turns.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
📊 Coverage ReportOverall Coverage: 92% Diff: origin/feat/multiturn-megabranch...HEAD
Summary
Line-by-lineView line-by-line diff coverageapp/desktop/studio_server/copilot_api.pyLines 499-507 499 spec_name=request.name,
500 )
501 task_runs = dataset_runs.task_runs
502 for run in task_runs:
! 503 run.parent = task
504 models_to_save.extend(task_runs)
505
506 # Snapshot the generation config on the Spec (single-turn only).
507 topic_cfg = request.sdg_session_config.topic_generation_configLines 554-563 554
555 for run in task_runs:
556 run.save_to_file()
557 saved_models.append(run)
! 558 if dataset_runs is not None:
! 559 dataset_runs.save_pending_feedback(run)
560
561 spec.save_to_file()
562 saved_models.append(spec)Lines 576-585 576 except Exception:
577 # Reverse any leaf tags we added in this run before deleting the
578 # saved models, so a failed multi-turn save doesn't leave orphan
579 # tags pointing at a now-deleted eval.
! 580 if tagged_leaves:
! 581 untag_multi_turn_chains_for_eval(tagged_leaves)
582 for model in reversed(saved_models):
583 try:
584 model.delete()
585 except Exception:app/desktop/studio_server/synthetic_user/client.pyLines 169-178 169 parts: list[str] = []
170 skipped = 0
171 for item in detail:
172 if not isinstance(item, ValidationError):
! 173 skipped += 1
! 174 continue
175 loc = ".".join(str(x) for x in item.loc)
176 parts.append(f"{loc}: {item.msg}")
177 if not parts:
178 # The SDK's HTTPValidationError.detail had items the SDK couldn'tLines 178-187 178 # The SDK's HTTPValidationError.detail had items the SDK couldn't
179 # parse as ValidationError — a shape we don't expect today. Log
180 # so we can spot the discrepancy if it ever appears in the wild,
181 # instead of silently returning the empty fallback.
! 182 if skipped:
! 183 logger.warning(
184 "HTTPValidationError carried %d non-ValidationError detail item(s); "
185 "raw detail repr: %r",
186 skipped,
187 detail,Lines 185-191 185 "raw detail repr: %r",
186 skipped,
187 detail,
188 )
! 189 return "Validation error (no detail)."
190 return "Validation error: " + "; ".join(parts)app/desktop/studio_server/utils/copilot_utils.pyLines 345-353 345 for leaf in leaves:
346 current = set(leaf.tags or [])
347 added = {eval_tag, golden_tag} - current
348 if not added:
! 349 continue
350 leaf.tags = sorted(current | added)
351 leaf.save_to_file()
352 if tagged_out is not None:
353 tagged_out.append((leaf, added))Lines 362-372 362 so pre-existing tags on the leaf are preserved. Best-effort: a per-leaf
363 save failure is logged and the loop continues — the original save error
364 that triggered cleanup is the one the user needs to see.
365 """
! 366 for leaf, added_tags in tagged_leaves:
! 367 try:
! 368 leaf.tags = sorted(set(leaf.tags or []) - added_tags)
! 369 leaf.save_to_file()
! 370 except Exception:
! 371 logger.exception(f"Failed to untag leaf {leaf.id} during cleanup")libs/core/kiln_ai/synthetic_user/drive_loop.pyLines 97-105 97 # Assert-loud on missing seed. An empty string would silently flow
98 # into the target adapter and surface as a confusing model-side error
99 # rather than a clean "the case is malformed" signal.
100 if not case.seed_prompt:
! 101 raise ValueError("case.seed_prompt must be a non-empty string")
102
103 user_msg: str = case.seed_prompt
104 prev_run: TaskRun | None = None
105 prev_trace: list[ChatCompletionMessageParam] | None = Nonelibs/core/kiln_ai/synthetic_user/driver.pyLines 114-122 114 swapped = role_swap(visible)
115 last = swapped[-1]
116 user_input = last["content"]
117 if not isinstance(user_input, str):
! 118 raise RuntimeError(
119 "synthetic user input must be a plain string after role_swap"
120 )
121
122 system_msg: ChatCompletionSystemMessageParam = {libs/core/kiln_ai/synthetic_user/role_swap.pyLines 41-49 41 # the target. Narrowing here lets us assign into the swapped wrapper
42 # type without a cast.
43 content = msg["content"]
44 if not isinstance(content, str):
! 45 raise ValueError(
46 f"role_swap requires string content for role {role!r}; "
47 f"got {type(content).__name__}"
48 )
49 if role == "user":libs/core/kiln_ai/synthetic_user/runner.pyLines 416-424 416 missing (defensive against fakes in unit tests that don't populate it).
417 """
418 usage = getattr(run, "cumulative_usage", None)
419 if usage is None:
! 420 return 0.0
421 return float(getattr(usage, "cost", None) or 0.0)
422
423
424 def _tag_leaf(leaf: TaskRun, batch_tag: str) -> None:
|
chiang-daniel
changed the title
… role_swap Tool-using targets emit assistant turns with content=None and tool_calls set — pure tool dispatches, not user-facing speech. Pre-this-fix, those hit role_swap's strict-content invariant and crashed the SU run. Gemini's suggestion (coerce None → "") would have let them through but degraded the SU LLM's conversation view to consecutive user turns with empty content — silently worse than the crash. The right place to filter is at the driver, next to the existing visible_message_roles filter — "what's visible to the SU" is the driver's responsibility. role_swap stays strict on None content (the trip wire for any caller bypassing the driver's filter). Filter predicate: drop assistant turns where content is None. Keep assistant turns that carry text alongside tool_calls — the text is user-facing speech the SU should respond to. Addresses gemini-code-assist comment on PR #1441 / role_swap.py without applying the suggested empty-string coercion. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…context Fix comment numbering in driver.py (4→5), correct "greedy" to "non-greedy" in parser.py, remove inaccurate drive-loop claim from studio_server __init__. Strip historical /respond migration references, remove app-layer concerns (SSE, @no_write_lock) from SDK-level docstrings, deduplicate cost-attribution explanations across driver/runner/drive_loop. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ask' into dchiang/multiturn-synthetic-user
Stray U+200B (zero-width space) between "disables/" and "spinners" in a comment tripped eslint no-irregular-whitespace. Likely a paste artifact from Leonard's recent commit; fixed in passing during the merge. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chiang-daniel
commented
Jun 3, 2026
|
|
||
| headers["Content-Type"] = "multipart/form-data; boundary=+++" | ||
|
|
||
| _kwargs["headers"] = headers |
Contributor
Author
There was a problem hiding this comment.
All the changes under /api_client are files copied from the new server SDK. No need to review those.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
libs/core/kiln_ai/synthetic_user/runner.py (1)
57-66: ⚖️ Poor tradeoff
TurnCompletedEvent.cumulative_costomits SU-driver spend whileCaseCompletedEvent.total_costincludes it.A live cost ticker driven off
cumulative_costwill undercount during turns, then jump up whencase_completedaddsresult.su_total_cost. This matches the documented "honest totals only at case end" intent, so it's not a bug — just flagging the per-turn vs per-case inconsistency in case the UI relies on a smooth running total. Threading the running SU cost intoon_turnwould remove the jump.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@libs/core/kiln_ai/synthetic_user/runner.py` around lines 57 - 66, TurnCompletedEvent.cumulative_cost currently excludes SU-driver spend while CaseCompletedEvent.total_cost includes it, causing per-turn cost undercounts then a jump at case completion; update the on-turn flow to thread the running SU cost into each TurnCompletedEvent so cumulative_cost reflects assistant+SU spend per turn (adjust the code paths that construct TurnCompletedEvent and any function handling on_turn to accept and pass the incremental su_running_cost), and ensure CaseCompletedEvent.total_cost still aggregates final su_total_cost so the live ticker remains smooth and consistent with the end-of-case total.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/web_ui/src/lib/api_schema.d.ts`:
- Around line 17079-17104: The OpenAPI docs currently advertise
stream_run_cases_batch
(stream_run_cases_batch_api_projects__project_id__tasks__task_id__multiturn_sdg_run_cases_batch_post)
as returning "application/json" but the route actually returns a
StreamingResponse with media_type="text/event-stream"; update the FastAPI route
in app/desktop/studio_server/multiturn_sdg_api.py to declare the 200 response
content type as "text/event-stream" (e.g., add responses={200: {"content":
{"text/event-stream": {"schema": {"type":"string"}}}}} or set
response_class/response_model metadata appropriately) so the OpenAPI spec
reflects SSE, then run app/web_ui/src/lib/generate_schema.sh to regenerate
app/web_ui/src/lib/api_schema.d.ts; do not manually edit the generated TS file.
---
Nitpick comments:
In `@libs/core/kiln_ai/synthetic_user/runner.py`:
- Around line 57-66: TurnCompletedEvent.cumulative_cost currently excludes
SU-driver spend while CaseCompletedEvent.total_cost includes it, causing
per-turn cost undercounts then a jump at case completion; update the on-turn
flow to thread the running SU cost into each TurnCompletedEvent so
cumulative_cost reflects assistant+SU spend per turn (adjust the code paths that
construct TurnCompletedEvent and any function handling on_turn to accept and
pass the incremental su_running_cost), and ensure CaseCompletedEvent.total_cost
still aggregates final su_total_cost so the live ticker remains smooth and
consistent with the end-of-case total.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dd8cddc7-7358-4d89-a388-06e6d09f5738
⛔ Files ignored due to path filters (20)
app/desktop/studio_server/api_client/kiln_ai_server_client/api/jobs/start_prompt_optimization_job_v1_jobs_prompt_optimization_job_start_post.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/api/jobs/start_sample_job_v1_jobs_sample_job_start_post.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/api/synthetic_user/__init__.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/api/synthetic_user/generate_v1_synthetic_user_generate_post.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/__init__.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/chat_completion_assistant_message_param_wrapper.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/chat_session_list_item.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/generate_synthetic_users_request.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/generate_synthetic_users_response.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/generate_v1_synthetic_user_generate_post_response_401.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/generate_v1_synthetic_user_generate_post_response_500.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/generate_v1_synthetic_user_generate_post_response_502.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/generate_v1_synthetic_user_generate_post_response_502_code.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/kiln_base_model.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/message_usage.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/synthetic_user_case.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/task_output.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/task_output_rating.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/task_run.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**app/desktop/studio_server/api_client/kiln_ai_server_client/models/usage.pyis excluded by!app/desktop/studio_server/api_client/kiln_ai_server_client/**
📒 Files selected for processing (26)
app/desktop/desktop_server.pyapp/desktop/studio_server/multiturn_sdg_api.pyapp/desktop/studio_server/synthetic_user/__init__.pyapp/desktop/studio_server/synthetic_user/client.pyapp/desktop/studio_server/synthetic_user/test_client.pyapp/desktop/studio_server/test_multiturn_sdg_api.pyapp/web_ui/src/lib/api_schema.d.tsapp/web_ui/src/lib/ui/conversation/multiturn_composer.sveltelibs/core/kiln_ai/synthetic_user/__init__.pylibs/core/kiln_ai/synthetic_user/case.pylibs/core/kiln_ai/synthetic_user/drive_loop.pylibs/core/kiln_ai/synthetic_user/driver.pylibs/core/kiln_ai/synthetic_user/models.pylibs/core/kiln_ai/synthetic_user/parser.pylibs/core/kiln_ai/synthetic_user/prompt.pylibs/core/kiln_ai/synthetic_user/role_swap.pylibs/core/kiln_ai/synthetic_user/runner.pylibs/core/kiln_ai/synthetic_user/test_case.pylibs/core/kiln_ai/synthetic_user/test_drive_loop.pylibs/core/kiln_ai/synthetic_user/test_driver.pylibs/core/kiln_ai/synthetic_user/test_models.pylibs/core/kiln_ai/synthetic_user/test_parser.pylibs/core/kiln_ai/synthetic_user/test_prompt.pylibs/core/kiln_ai/synthetic_user/test_role_swap.pylibs/core/kiln_ai/synthetic_user/test_runner.pylibs/server/kiln_server/server.py
Comment on lines
+17079
to
+17104
| stream_run_cases_batch_api_projects__project_id__tasks__task_id__multiturn_sdg_run_cases_batch_post: { | ||
| parameters: { | ||
| query?: never; | ||
| header?: never; | ||
| path: { | ||
| /** @description ID of the project containing the target task. */ | ||
| project_id: string; | ||
| /** @description ID of the target task. Must be a multi-turn task. */ | ||
| task_id: string; | ||
| }; | ||
| cookie?: never; | ||
| }; | ||
| requestBody: { | ||
| content: { | ||
| "application/json": components["schemas"]["RunCasesBatchApiInput"]; | ||
| }; | ||
| }; | ||
| responses: { | ||
| /** @description Successful Response */ | ||
| 200: { | ||
| headers: { | ||
| [name: string]: unknown; | ||
| }; | ||
| content: { | ||
| "application/json": unknown; | ||
| }; |
Contributor
There was a problem hiding this comment.
run_cases_batch response media type is mis-modeled as JSON instead of SSE.
stream_run_cases_batch is typed with 200 -> application/json, but the backend route returns StreamingResponse(..., media_type="text/event-stream") (see app/desktop/studio_server/multiturn_sdg_api.py). This weakens the generated client contract for streaming and can break typed frontend consumption.
Please update the backend route OpenAPI metadata/response docs to advertise text/event-stream, then regenerate app/web_ui/src/lib/api_schema.d.ts via app/web_ui/src/lib/generate_schema.sh rather than editing this file directly.
Based on learnings: "app/web_ui/src/lib/api_schema.d.ts is auto-generated by openapi-typescript; do not propose manual edits. Schema changes should be made in the FastAPI backend … then re-generate the TS types."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/web_ui/src/lib/api_schema.d.ts` around lines 17079 - 17104, The OpenAPI
docs currently advertise stream_run_cases_batch
(stream_run_cases_batch_api_projects__project_id__tasks__task_id__multiturn_sdg_run_cases_batch_post)
as returning "application/json" but the route actually returns a
StreamingResponse with media_type="text/event-stream"; update the FastAPI route
in app/desktop/studio_server/multiturn_sdg_api.py to declare the 200 response
content type as "text/event-stream" (e.g., add responses={200: {"content":
{"text/event-stream": {"schema": {"type":"string"}}}}} or set
response_class/response_model metadata appropriately) so the OpenAPI spec
reflects SSE, then run app/web_ui/src/lib/generate_schema.sh to regenerate
app/web_ui/src/lib/api_schema.d.ts; do not manually edit the generated TS file.
Extends spec_with_copilot to handle multi-turn synthetic-user batches. When the request carries a `multi_turn.batch_tag`, the endpoint: - finds existing chain leaves tagged synthetic_user_batch:<batch_tag> - applies the spec's eval + golden filter tags to them - creates Eval with evaluation_data_type=full_trace and train_set_filter_id=None - skips example synthesis and TaskRun creation A request-shape validator enforces mutual exclusion with sdg_session_config and requires evaluate_full_trace=True for the multi-turn path. Also adds classify_spec_description as a stub endpoint that returns 501 until the kiln_server classifier ships. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds the v2 eval builder at /specs_v2/{project_id}/{task_id} — a single-
page wizard that simplifies v1's 9-template carousel + multi-field form
into one description box → Q&A → editable refine → generate → review →
save flow. Sidebar entry under "Evals V2 (Beta)".
Reuses v1 spec_builder components (Questions, RefineSpec, ReviewExamples)
on the shared screens so the look-and-feel stays in sync with v1. The
multi-turn branch at Step 4 + 5 + 6 has its own custom UI for chat-trace
review and ties into the new spec_with_copilot multi-turn save path.
Step 1 calls classify_spec_description (currently 501) with a graceful
fallback to "issue" defaults. Multi-turn generation (Step 4) is a stub —
real run_cases_batch SSE wiring is still pending; surface a clear error
until that lands.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 4 multi-turn now drives the real run_cases_batch endpoint instead
of simulating with hardcoded chains:
1. Resolve the task's default run config → target_run_config (the
drive loop invokes the agent the same way a normal task run would).
2. POST /multiturn_sdg/generate_cases → N=10 synthetic-user cases.
3. POST /multiturn_sdg/run_cases_batch as SSE; consume the stream via
fetch + ReadableStream (same pattern as streaming_chat.ts since the
endpoint is POST and EventSource is GET-only).
4. Dispatch BatchEvents into component state: batch_started seeds the
batch_tag used by the multi-turn save, turn_completed updates the
cumulative trace per case, case_completed appends a Chain to the
review cards, batch_completed advances to Step 5.
SU driver model is hardcoded for MVP (claude_4_5_haiku via openrouter)
per design.md; surfacing the choice in the UI is deferred.
With this in place the multi-turn save path uses a real batch_tag and
the previous "no real chains, route through single-turn pipeline"
stopgap can be removed. The save step is now a single-path call to
spec_with_copilot with multi_turn={batch_tag}.
Sidebar entry now carries a comment noting that the v1 Evals tab is
intended to be removed once v2 ships GA.
Also adds get_task_composite_id to the edit_task test's $lib/stores
mock so the new transitive import doesn't trip an incomplete-mock
warning during test runs.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…cription Two related bugs surfaced when walking the v2 builder without a real classifier (which currently 501s): 1. Step 1's free-text `description` never flowed into `property_values.issue_description`. Step 3's Refine screen rendered the "Original" column from `property_values`, so issue_description showed up empty even though the user had typed it in Step 1. The missing required field then silently blocked the form's Continue button (RefineSpec validator). 2. `refine_submitting` was set true by FormContainer on submit but never reset by the v2 handler. After a successful click the flag stayed true; if the user navigated back to Step 3 the button would be permanently disabled. For (1): seed `property_values.issue_description = description` at the start of classify_then_continue, before the classifier call. Real classifier response (when it ships) still overwrites; on 501 fallback we keep the user's input as the issue description. For (2): clear `refine_submitting` before advancing — the handler doesn't await any network call so there's no submitting state to preserve. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tasks created via the new-task wizard don't auto-set default_run_config_id, so first-time multi-turn users were blocked at Step 4 with "Task has no default run config — set one in task settings" even though the task had a perfectly usable run config available. Prefer the default when set; otherwise pick the first available config. Only error when the task has zero configs (genuinely unrunnable). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
v1's RefineSpec component (which v2 was reusing for both modes) is overkill for multi-turn: examples fields don't apply (the synthetic- user chains from Step 4 are the real "examples"), and the Original / Refined two-column diff plus duplicate name inputs add UX friction when only the description is meaningfully editable. For multi-turn tasks, render a stripped-down variant: one editable name input, one editable description textarea (with the refinement reason shown inline when present), and unincorporated-feedback note if any. Single-turn keeps using v1's RefineSpec. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 4 multi-turn does two sequential operations: first a single LLM
call to /multiturn_sdg/generate_cases (can take 5-15s for 10 cases),
then the longer streaming run_cases_batch SSE batch (minutes). The UI
was showing the same "0 of 10 ready" message during the case-gen call,
which is misleading — no chains are running yet, the copilot is still
authoring the personas.
Add a multi_turn_phase state ("idle" | "generating_cases" |
"running_batch") and branch the Step 4 copy on it. Title + subtitle +
status line all reflect what's actually happening behind the spinner.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Step 5 multi-turn was letting users hit Save with chain_verdicts still null — the golden ratings ended up empty and the underlying eval got created with no validated examples. Disable the button until every chain has been marked pass or fail; add a tooltip on the disabled state so the user understands what's blocking. Single-turn already enforces this via v1's ReviewExamples component (its submit_disabled = !all_feedback_aligned && !all_examples_reviewed) so no change needed there. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Multi-turn save was calling /api/copilot/clarify_spec just to harvest the judge_result field — clarify_spec also runs topic + input + output example generation that we throw away, costing 5-10 minutes per save. Beyond perf, server-side judge generation has a deeper problem (per Steve): the judge model has to come from the server's model registry, which can't include the user's locally-hosted models, custom fine-tunes, or models behind keys the server doesn't have. So judge model choice belongs on the client. Synthesize the judge config client-side: default model gpt_4o via openrouter, with a generic conversation-trace-evaluation prompt that inlines the spec definition. Extracted into app/web_ui/src/lib/eval/default_judge.ts so studio_server, future CLI tooling, or a UI judge-model picker can share the same defaults. Trade-off acknowledged inline: the templated prompt is weaker than clarify_spec's LLM-authored spec-specific rubric (which cited concrete red flags by name). The path forward is a UI picker that lets users override both model and prompt; this commit just stops the unbounded wait. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Several UI issues called out from internal review:
- v2 was constrained to max-w-3xl (768px) across every step; v1 widens
to 900/1400px per step. Mirror v1's getPageClass-style logic via
page_max_w_for(step): 1400px for review and refine-with-suggestions
(single-turn), 900px otherwise. Fixes the "v2 not using full
real-estate" feel and lets v1's reused Questions/RefineSpec/
ReviewExamples components render at their intended widths.
- Move per-step title + subtitle into AppPage props (title /
subtitle), matching v1's page header pattern. Removes the duplicate
in-body h1 + paragraph that lived in each step's markup. Titles
audited to match v1's tone ("Create Eval", "Clarify Eval", "Refine
Eval", "Review Conversations" / "Review Examples", "Creating Eval").
- Drop the redundant "Case N" persona_summary line on multi-turn
review cards — "Conversation N of N" already conveys that.
- Add filename_string_short_validator to the multi-turn Step 3 Eval
Name input so the user gets immediate red-text feedback if the name
exceeds 32 chars (the FilenameStringShort limit on EvalOutputScore.name)
or violates the other filename rules. Previously a too-long name
would only fail server-side with a 422 at save time.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
6 tasks
… rollback fix - Move v2 builder to /specs/[project_id]/[task_id]/builder (drop /specs_v2/ URL). - Pro-gate the new builder route via CopilotRequiredCard. - Flip v1 listing's "Create Eval" CTA: Pro users land on the v2 builder; non-Pro keeps the existing select_workflow flow. - Replace "Evals V2 Beta" sidebar entry with a temporary "Evals Legacy" entry (TODO-marked for removal post-GA) for side-by-side comparison. - Fix rollback gap: multi-turn save failures now untag any leaves we added tags to in this run, preserving pre-existing tags. - Surface guardrails: multi-turn comparison notice on the eval detail page, Step 1 beta hint, default-run-config fallback notice. - Posthog events on the v2 builder flow (open, step entered, save success/error, CTA branch). - Apply CR audit fixes (critical + moderate) on stale or false comments. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
- Match v1 spec_builder's width pattern: wrap the entire AppPage in the page_max_w div so title and body share the same constraint. - Replace raw alert/info divs with the shared Warning component (6 sites in builder + 1 in compare_run_configs). - Standardize Back/Cancel buttons to btn-ghost btn-sm. - Wire classify_error through FormElement's error_message prop. - Bump step indicator from text-xs to text-sm to match v1. - Drop the "Beta: every spec treated as Issue" hint — coordinate with Mike via the bug bash announcement instead. - Add v1's AbortController plumbing (abort_copilot_request, new_copilot_abort_signal, is_abort_error) and onDestroy cleanup. Long-running Copilot calls pass the signal, Back buttons cancel in-flight requests, catch blocks silently ignore AbortError. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace Step 5's vertical card stack with a focused paginator: one conversation at a time, dot-row navigator at the top, persona + verdict buttons inside the card, single bottom action bar. - Pass auto-advances to the next un-reviewed conversation; Fail keeps the user on the current case and focuses the feedback input, since the judge prompt needs the reason to learn from. - Save is gated until every chain has a verdict AND every failed chain has a non-empty reason. Tooltip explains what's missing. - Single bottom action bar owns Back/Prev/Next/Save so the user sees one set of buttons instead of two stacked rows. - Dots are small solid markers with numbered labels below, connected by solid lines (matches DaisyUI .steps line convention). - Feedback wording mirrors v1 review_examples: "Describe why this fails" / "Describe why this passes (optional)". Adds a DEV-ONLY mock route at /dev_mock_review for iterating on the paginator without running through the full eval-builder flow. TODO-marked for removal post-bash. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Backfill the checked-in agent policy annotation files for the two new multiturn_sdg routes (generate_cases, run_cases_batch) so the API bindings CI check passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@libs/server/kiln_server/utils/agent_checks/annotations/post_api_projects_project_id_tasks_task_id_multiturn_sdg_run_cases_batch.json`:
- Line 7: Update the approval_description for the multi-turn batch endpoint to
mention that approving it will persist multi-turn TaskRun chains, not just
invoke the target and SU driver models at a cost. Locate the annotation in the
post_api_projects_project_id_tasks_task_id_multiturn_sdg_run_cases_batch JSON
and revise the wording so users understand the endpoint writes run data as a
side effect, alongside the existing model-cost warning.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 322004df-e13c-4fb6-a648-54892430c086
📒 Files selected for processing (2)
libs/server/kiln_server/utils/agent_checks/annotations/post_api_projects_project_id_tasks_task_id_multiturn_sdg_generate_cases.jsonlibs/server/kiln_server/utils/agent_checks/annotations/post_api_projects_project_id_tasks_task_id_multiturn_sdg_run_cases_batch.json
| "agent_policy": { | ||
| "permission": "allow", | ||
| "requires_approval": true, | ||
| "approval_description": "Run a multi-turn synthetic-user batch? Invokes the target model and the SU driver model for several turns per case (cost)." |
Contributor
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Mention the persisted TaskRun side effect in the approval text.
This approval prompt only calls out model cost, but the PR objective says batch execution persists multi-turn TaskRun chains. Users approving an agent action should see that this endpoint writes run data, not just spends tokens.
Suggested wording
- "approval_description": "Run a multi-turn synthetic-user batch? Invokes the target model and the SU driver model for several turns per case (cost)."
+ "approval_description": "Run a multi-turn synthetic-user batch? Invokes the target model and synthetic-user driver model for several turns per case, then persists the resulting TaskRun chains (cost)."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "approval_description": "Run a multi-turn synthetic-user batch? Invokes the target model and the SU driver model for several turns per case (cost)." | |
| "approval_description": "Run a multi-turn synthetic-user batch? Invokes the target model and synthetic-user driver model for several turns per case, then persists the resulting TaskRun chains (cost)." |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@libs/server/kiln_server/utils/agent_checks/annotations/post_api_projects_project_id_tasks_task_id_multiturn_sdg_run_cases_batch.json`
at line 7, Update the approval_description for the multi-turn batch endpoint to
mention that approving it will persist multi-turn TaskRun chains, not just
invoke the target and SU driver models at a cost. Locate the annotation in the
post_api_projects_project_id_tasks_task_id_multiturn_sdg_run_cases_batch JSON
and revise the wording so users understand the endpoint writes run data as a
side effect, alongside the existing model-cost warning.
feat: Multiturn eval builder
chiang-daniel
changed the title
The SDK re-vendor reintroduced a hardcoded "Content-Type: multipart/form-data; boundary=+++" on the prompt optimization endpoint, an openapi-python-client multipart bug main had already fixed. The placeholder boundary prevents httpx from negotiating its own, corrupting the multipart body. Remove the line to match main and satisfy test_multipart_boundary. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/desktop/studio_server/copilot_api.py`:
- Around line 122-132: The multi-turn save flow currently tags matching leaves
as golden without persisting any review feedback, so the reviewed verdicts are
lost. Update the save request/model around MultiTurnSaveInfo and the multi-turn
endpoint that handles reviewed_examples to accept reviewed-chain data keyed by
leaf_run_id and case_index, persist Feedback for those chains first, and only
then apply golden_tag in the save path. Make sure the review-builder path that
currently sends reviewed_examples: [] is wired to pass the actual ratings before
the tags are applied.
In `@app/web_ui/src/routes/`(app)/dev_mock_review/+page.svelte:
- Around line 1-8: The dev-only mock route is currently exposed as a real
`(app)` page, so users can access the hardcoded conversations and placeholder
claims. Update the `+page.svelte` route for `dev_mock_review` to be gated behind
a dev-only flag/build condition, or remove the route entirely before merge; make
sure the `MultiTurnReviewPaginator` mock content is not reachable in production.
In
`@app/web_ui/src/routes/`(app)/specs/[project_id]/[task_id]/builder/+page.svelte:
- Around line 154-167: Clear the downstream wizard state at the start of
classify_then_continue so a changed description triggers fresh Step 2/3 data
instead of reusing stale results. In the classify_then_continue flow in
builder/+page.svelte, reset question_set and any dependent state such as
question_answers, refined_spec, and related step state before calling
load_questions or continuing classification, so the existing question_set no
longer short-circuits reloading after the user edits description and retries.
- Around line 451-458: Reset the stale review state when starting a new
multi-turn generation run in on_generate_multi_turn, because clearing
multi_turn_chains alone leaves chain_verdicts from the previous attempt behind.
Update the reset block to also clear chain_verdicts (and any related
verdict-tracking state if present) alongside multi_turn_progress,
multi_turn_batch_tag, and multi_turn_phase so a retry cannot reuse old verdicts
when the new run has the same chain count.
- Around line 360-377: The single-turn example generator is using the stale
initial description instead of the refined spec state. Update
on_generate_single_turn() in the builder page to send the refined property
values/spec content that is actually persisted after Step 3, rather than
description, so the clarifier request matches the final spec. Use the existing
refined_property_values flow in the same component to locate the correct source
of truth before building the client.POST("/api/copilot/clarify_spec") payload.
In
`@app/web_ui/src/routes/`(app)/specs/[project_id]/[task_id]/builder/multi_turn_review_paginator.svelte:
- Around line 161-172: Guard the feedback input rendering in
multi_turn_review_paginator.svelte so it only appears when the current verdict
entry exists and has a non-null verdict. Update the {`#if`} around current_verdict
and verdicts[current_index].feedback to explicitly check that current_verdict is
defined before dereferencing/binding. Use the current_verdict and
verdicts[current_index] bindings as the key symbols to locate the block, and
keep bind:this={feedback_input} only inside the safe condition.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bac68916-0454-4631-8ce9-44b33d1c7e55
📒 Files selected for processing (14)
app/desktop/studio_server/copilot_api.pyapp/desktop/studio_server/test_copilot_api.pyapp/desktop/studio_server/utils/copilot_utils.pyapp/web_ui/src/lib/api_schema.d.tsapp/web_ui/src/lib/eval/default_judge.tsapp/web_ui/src/routes/(app)/+layout.svelteapp/web_ui/src/routes/(app)/dev_mock_review/+page.svelteapp/web_ui/src/routes/(app)/dev_mock_review/+page.tsapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/[spec_id]/[eval_id]/compare_run_configs/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/builder/+page.svelteapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/builder/+page.tsapp/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/builder/multi_turn_review_paginator.svelteapp/web_ui/src/routes/(fullscreen)/setup/(setup)/create_task/edit_task.test.ts
✅ Files skipped from review due to trivial changes (4)
- app/web_ui/src/routes/(app)/specs/[project_id]/[task_id]/builder/+page.ts
- app/web_ui/src/routes/(app)/+layout.svelte
- app/web_ui/src/routes/(app)/dev_mock_review/+page.ts
- app/web_ui/src/lib/api_schema.d.ts
Comment on lines
+122
to
+132
| class MultiTurnSaveInfo(BaseModel): | ||
| """Identifies an existing multi-turn synthetic-user batch to turn into an Eval. | ||
| The endpoint walks chains tagged with this batch_tag and applies eval/golden | ||
| filter tags instead of generating new examples. | ||
| """ | ||
|
|
||
| batch_tag: str = Field( | ||
| description="The batch_tag emitted by the multi-turn synthetic-user runner " | ||
| "(see kiln_ai.synthetic_user.runner). Identifies the set of conversation " | ||
| "chains already persisted to disk that this Eval should evaluate." | ||
| ) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Persist reviewed chain ratings before tagging leaves as golden.
MultiTurnSaveInfo only carries batch_tag, and the save path tags matching leaves with golden_tag without creating any Feedback. The builder’s multi-turn review gates on pass/fail verdicts, but its save request sends reviewed_examples: [], so the golden set becomes unrated and the reviewer input is discarded. Add a reviewed-chain payload keyed by leaf_run_id/case_index and persist feedback before applying golden_tag, or avoid tagging these leaves as golden until ratings exist.
Also applies to: 564-575
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/desktop/studio_server/copilot_api.py` around lines 122 - 132, The
multi-turn save flow currently tags matching leaves as golden without persisting
any review feedback, so the reviewed verdicts are lost. Update the save
request/model around MultiTurnSaveInfo and the multi-turn endpoint that handles
reviewed_examples to accept reviewed-chain data keyed by leaf_run_id and
case_index, persist Feedback for those chains first, and only then apply
golden_tag in the save path. Make sure the review-builder path that currently
sends reviewed_examples: [] is wired to pass the actual ratings before the tags
are applied.
Comment on lines
+1
to
+8
| <!-- | ||
| DEV-ONLY mock page for iterating on the multi-turn review paginator | ||
| without running through the full eval-builder flow each time. Hardcodes | ||
| 10 plausible synthetic-user conversations and renders them through the | ||
| same MultiTurnReviewPaginator the v2 builder uses. | ||
|
|
||
| TODO(eval-v2): delete this route once the paginator UI is locked in. | ||
| Reach via /dev_mock_review. |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don’t ship the dev-only mock route unguarded.
/dev_mock_review is a real route under (app), so the hardcoded mock conversations and product claims can become user-visible. Remove it before merge or gate it behind a dev-only flag/build condition.
I can help turn this into a dev-only guard or open a cleanup issue if you want.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/web_ui/src/routes/`(app)/dev_mock_review/+page.svelte around lines 1 - 8,
The dev-only mock route is currently exposed as a real `(app)` page, so users
can access the hardcoded conversations and placeholder claims. Update the
`+page.svelte` route for `dev_mock_review` to be gated behind a dev-only
flag/build condition, or remove the route entirely before merge; make sure the
`MultiTurnReviewPaginator` mock content is not reachable in production.
Comment on lines
+154
to
+167
| async function classify_then_continue() { | ||
| classifying = true | ||
| classify_error = null | ||
| try { | ||
| // Seed property_values.issue_description from the free-text description | ||
| // up front. This is the fallback shape for the "issue" default — when | ||
| // the classifier ships, it'll overwrite below. Done here so Step 3's | ||
| // Refine reflects what the user typed in Step 1 (and Step 2's | ||
| // refine_spec_with_question_answers has something to refine from), | ||
| // even if classification fails. | ||
| property_values = { | ||
| ...property_values, | ||
| issue_description: description, | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear downstream wizard state when reclassifying.
If the user goes back, edits description, and continues again, the existing question_set prevents load_questions() from running, so Step 2/3 can reuse stale questions, answers, and refinements from the previous description.
Proposed fix
async function classify_then_continue() {
classifying = true
classify_error = null
try {
+ question_set = null
+ selections = []
+ other_texts = []
+ refined_property_values = {}
+ suggested_edits = {}
+ not_incorporated_feedback = ""
+
// Seed property_values.issue_description from the free-text descriptionAlso applies to: 841-844
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/web_ui/src/routes/`(app)/specs/[project_id]/[task_id]/builder/+page.svelte
around lines 154 - 167, Clear the downstream wizard state at the start of
classify_then_continue so a changed description triggers fresh Step 2/3 data
instead of reusing stale results. In the classify_then_continue flow in
builder/+page.svelte, reset question_set and any dependent state such as
question_answers, refined_spec, and related step state before calling
load_questions or continuing classification, so the existing question_set no
longer short-circuits reloading after the user edits description and retries.
Comment on lines
+360
to
+377
| async function on_generate_single_turn() { | ||
| generation_loading = true | ||
| generation_error = null | ||
| try { | ||
| const { data, error } = await client.POST("/api/copilot/clarify_spec", { | ||
| body: { | ||
| target_task_info: { | ||
| task_prompt: task?.instruction ?? "", | ||
| task_input_schema: "", | ||
| task_output_schema: "", | ||
| }, | ||
| target_specification: description, | ||
| num_samples_per_topic: 10, | ||
| num_topics: 10, | ||
| providers: ["openrouter"], | ||
| num_exemplars: 10, | ||
| }, | ||
| signal: new_copilot_abort_signal(), |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Generate single-turn examples from the refined spec, not the initial description.
After Step 3, description can be stale relative to refined_property_values, so generated examples may not match the spec that is later saved.
Proposed fix
async function on_generate_single_turn() {
generation_loading = true
generation_error = null
try {
+ const target_specification =
+ (refined_property_values.issue_description as string | null) ??
+ description
const { data, error } = await client.POST("/api/copilot/clarify_spec", {
body: {
@@
- target_specification: description,
+ target_specification,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function on_generate_single_turn() { | |
| generation_loading = true | |
| generation_error = null | |
| try { | |
| const { data, error } = await client.POST("/api/copilot/clarify_spec", { | |
| body: { | |
| target_task_info: { | |
| task_prompt: task?.instruction ?? "", | |
| task_input_schema: "", | |
| task_output_schema: "", | |
| }, | |
| target_specification: description, | |
| num_samples_per_topic: 10, | |
| num_topics: 10, | |
| providers: ["openrouter"], | |
| num_exemplars: 10, | |
| }, | |
| signal: new_copilot_abort_signal(), | |
| async function on_generate_single_turn() { | |
| generation_loading = true | |
| generation_error = null | |
| try { | |
| const target_specification = | |
| (refined_property_values.issue_description as string | null) ?? | |
| description | |
| const { data, error } = await client.POST("/api/copilot/clarify_spec", { | |
| body: { | |
| target_task_info: { | |
| task_prompt: task?.instruction ?? "", | |
| task_input_schema: "", | |
| task_output_schema: "", | |
| }, | |
| target_specification, | |
| num_samples_per_topic: 10, | |
| num_topics: 10, | |
| providers: ["openrouter"], | |
| num_exemplars: 10, | |
| }, | |
| signal: new_copilot_abort_signal(), |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/web_ui/src/routes/`(app)/specs/[project_id]/[task_id]/builder/+page.svelte
around lines 360 - 377, The single-turn example generator is using the stale
initial description instead of the refined spec state. Update
on_generate_single_turn() in the builder page to send the refined property
values/spec content that is actually persisted after Step 3, rather than
description, so the clarifier request matches the final spec. Use the existing
refined_property_values flow in the same component to locate the correct source
of truth before building the client.POST("/api/copilot/clarify_spec") payload.
Comment on lines
+451
to
+458
| async function on_generate_multi_turn() { | ||
| generation_loading = true | ||
| generation_error = null | ||
| multi_turn_progress = 0 | ||
| multi_turn_chains = [] | ||
| multi_turn_batch_tag = null | ||
| multi_turn_phase = "idle" | ||
|
|
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reset review verdicts when starting a new multi-turn run.
multi_turn_chains is cleared, but chain_verdicts is not. If a retry produces the same number of chains, the reactive length check preserves old verdicts and can enable saving without reviewing the new conversations.
Proposed fix
multi_turn_progress = 0
multi_turn_chains = []
+chain_verdicts = []
multi_turn_batch_tag = null
multi_turn_phase = "idle"
+multi_turn_fallback_run_config_name = null📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function on_generate_multi_turn() { | |
| generation_loading = true | |
| generation_error = null | |
| multi_turn_progress = 0 | |
| multi_turn_chains = [] | |
| multi_turn_batch_tag = null | |
| multi_turn_phase = "idle" | |
| async function on_generate_multi_turn() { | |
| generation_loading = true | |
| generation_error = null | |
| multi_turn_progress = 0 | |
| multi_turn_chains = [] | |
| chain_verdicts = [] | |
| multi_turn_batch_tag = null | |
| multi_turn_phase = "idle" | |
| multi_turn_fallback_run_config_name = null |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/web_ui/src/routes/`(app)/specs/[project_id]/[task_id]/builder/+page.svelte
around lines 451 - 458, Reset the stale review state when starting a new
multi-turn generation run in on_generate_multi_turn, because clearing
multi_turn_chains alone leaves chain_verdicts from the previous attempt behind.
Update the reset block to also clear chain_verdicts (and any related
verdict-tracking state if present) alongside multi_turn_progress,
multi_turn_batch_tag, and multi_turn_phase so a retry cannot reuse old verdicts
when the new run has the same chain count.
Comment on lines
+161
to
+172
| {#if current_verdict?.verdict !== null} | ||
| <input | ||
| type="text" | ||
| class="input input-bordered input-sm mt-4 {current_needs_reason | ||
| ? 'input-error' | ||
| : ''}" | ||
| placeholder={current_verdict?.verdict === "fail" | ||
| ? "Describe why this fails" | ||
| : "Describe why this passes (optional)"} | ||
| bind:value={verdicts[current_index].feedback} | ||
| bind:this={feedback_input} | ||
| /> |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Guard undefined verdict entries before rendering/binding feedback.
Line 161 uses current_verdict?.verdict !== null, which is true when current_verdict is undefined. That allows Line 170 to bind verdicts[current_index].feedback and can crash at runtime.
Suggested fix
- {`#if` current_verdict?.verdict !== null}
+ {`#if` current_verdict && current_verdict.verdict !== null}
<input
type="text"
class="input input-bordered input-sm mt-4 {current_needs_reason
? 'input-error'
: ''}"
placeholder={current_verdict?.verdict === "fail"
? "Describe why this fails"
: "Describe why this passes (optional)"}
bind:value={verdicts[current_index].feedback}
bind:this={feedback_input}
/>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {#if current_verdict?.verdict !== null} | |
| <input | |
| type="text" | |
| class="input input-bordered input-sm mt-4 {current_needs_reason | |
| ? 'input-error' | |
| : ''}" | |
| placeholder={current_verdict?.verdict === "fail" | |
| ? "Describe why this fails" | |
| : "Describe why this passes (optional)"} | |
| bind:value={verdicts[current_index].feedback} | |
| bind:this={feedback_input} | |
| /> | |
| {`#if` current_verdict && current_verdict.verdict !== null} | |
| <input | |
| type="text" | |
| class="input input-bordered input-sm mt-4 {current_needs_reason | |
| ? 'input-error' | |
| : ''}" | |
| placeholder={current_verdict?.verdict === "fail" | |
| ? "Describe why this fails" | |
| : "Describe why this passes (optional)"} | |
| bind:value={verdicts[current_index].feedback} | |
| bind:this={feedback_input} | |
| /> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@app/web_ui/src/routes/`(app)/specs/[project_id]/[task_id]/builder/multi_turn_review_paginator.svelte
around lines 161 - 172, Guard the feedback input rendering in
multi_turn_review_paginator.svelte so it only appears when the current verdict
entry exists and has a non-null verdict. Update the {`#if`} around current_verdict
and verdicts[current_index].feedback to explicitly check that current_verdict is
defined before dereferencing/binding. Use the current_verdict and
verdicts[current_index] bindings as the key symbols to locate the block, and
keep bind:this={feedback_input} only inside the safe condition.
…description Combining branches pulled in the new /api/copilot/classify_spec_description endpoint; add its checked-in agent policy annotation so the API bindings CI check passes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ask' into dchiang/multiturn-synthetic-user # Conflicts: # app/web_ui/src/lib/api_schema.d.ts
The v1 listing page had pre-v2 guards that hid the "Create Eval" CTA and replaced the body with "Evals are not supported for multi-turn tasks." when task.turn_mode === "multiturn". Those guards were defensive code from when v1's spec_builder couldn't handle multi-turn; v2's builder is multi-turn-aware so they no longer apply. Drop the guards and the now-dead task-load plumbing they fed (is_multiturn reactive, task / task_loading state, load_task_for_page, and the Task / load_task / Warning imports). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Base automatically changed from
leonard/kil-632-feat-multiturn-task
to
feat/multiturn-megabranch
June 25, 2026 14:13
…md-Enter Bucket D of Steve's eval-builder bug-bash feedback: - Reuse v1 Analyzing/Refining/Questioning/Saving animations on Steps 2-4 and 6 instead of bare dot-spinners; multi-turn keeps its live progress count via a reactive caption. - Add a 'Create manually' link under Step 1 to the legacy template flow. - Cmd/Ctrl-Enter on the bespoke primary buttons (Step 1, multi-turn refine, multi-turn save); FormContainer-backed steps already had it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Multi-turn Step 4 only bumped its count on case_completed, so progress sat still through each concurrency-limited wave (the runner is already a 4-wide semaphore pool) then jumped — reading as serial. Count turn_completed events against NUM_CASES*TURNS_PER_CASE and show a progress bar so it climbs steadily. Perception fix only; no concurrency change (raising it risks 429s with no retry policy yet). Addresses Steve feedback #15. #16 verified separately (classify 501 falls back to 'issue' silently, no leaked toast) — no code change needed. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… guard) Replaces the ad-hoc per-step Back buttons with Svelte shallow routing: each step transition records the step in history.state (goto_step pushes, replace_step swaps transient loading steps), and a popstate-driven reactive restores the step on browser Back/Forward — aborting any in-flight request. Because navigation stays within the single /builder route, the component stays mounted and no in-progress state is lost. Fixes Steve feedback #3 (use browser back, remove our own back buttons), #4 (browser-back data loss, wrong-step jump, stuck spinner) and #12 (beforeunload warning on unsaved state). The step is just a value in history.state, so this survives future changes to the set of steps. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
kiln_ai.synthetic_user.runner—drive_case+run_cases_batchSyntheticUserCasecontractSyntheticUserClientwrapping kiln_server/v1/synthetic_user/generategenerate_cases(sync) +run_cases_batch(SSE)Pipeline
/generate(pro-gated, kiln-AI keys)synthetic_user_case+synthetic_user_batch:<tag>asyncio.Semaphore(4); stream BatchEvents over SSENotable
total_costhonestly sums target adapter + SU driver spendNUM_CASES_MAX=10,MAX_TURNS_DEFAULT=5,CONCURRENCY=4Flow
Test plan
_smoke.py, untracked): 3 hand-crafted cases → 3 persisted chains, $0.04 totalRelated Issues
Contributor License Agreement
I, @, confirm that I have read and agree to the Contributors License Agreement.
Checklists