feat: assistant auto-mode + the eval/judge job backend (context gauge, chat resilience, queued messages)#1517
Draft
leonardmq wants to merge 66 commits into
Draft
feat: assistant auto-mode + the eval/judge job backend (context gauge, chat resilience, queued messages)#1517leonardmq wants to merge 66 commits into
leonardmq wants to merge 66 commits into
Conversation
Add an EvalJobWorker that wraps the existing EvalRunner so an eval can run
in the background through the job system. Expose a typed, non-streaming
kickoff endpoint POST /api/jobs/evals/run for agents (allow + requires
approval); poll GET /api/jobs/{id} for progress/result instead of SSE. The
endpoint uses a two-segment path so it can never be shadowed by the generic
POST /api/jobs/{type} route.
Flip the UI's SSE eval-run endpoints (run_comparison, run_calibration) to
agent-forbidden, since agents should use the background job API instead.
Also fix a reconcile bug surfaced by review: a worker that can't derive its
error count from entities (failed eval items leave no EvalRun) now returns
JobDerivedState.error=None, and _apply_derived keeps the live reported error
count instead of clobbering it to 0 on reconcile (mirrors total/message).
Regenerated agent-check annotations and the OpenAPI TS schema.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013QNY986xT2VCm7xngUg4tF
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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:
WalkthroughThe PR implements two major features: typed eval background jobs (with ChangesEval background jobs
Assistant auto-mode
Sequence DiagramsequenceDiagram
participant Browser
participant ChatStreamSession
participant AutoChatRegistry
participant AutoChatRunner
participant AutoChatEventBus
participant UpstreamBackend
Browser->>ChatStreamSession: POST /api/chat (enable_auto_mode tool call arrives)
ChatStreamSession-->>Browser: SSE auto-mode-consent-required
Browser->>AutoChatRegistry: POST /api/chat/auto/enable (seed)
AutoChatRegistry->>AutoChatRunner: start supervised burst
AutoChatRunner->>UpstreamBackend: POST /v1/chat (auto-execute tools each round)
UpstreamBackend-->>AutoChatRunner: SSE stream
AutoChatRunner->>AutoChatEventBus: publish bytes
Browser->>AutoChatEventBus: GET /api/chat/auto/{run_id}/events
AutoChatEventBus-->>Browser: replay buffer + live SSE
AutoChatRunner-->>AutoChatRegistry: settle IDLE or terminal
AutoChatRegistry->>AutoChatEventBus: publish auto-mode-idle/off marker
Estimated code review effort🎯 5 (Critical) | ⏱️ ~150 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ 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 a background worker (EvalJobWorker) and a new typed API endpoint (POST /api/jobs/evals/run) to allow agents to run evaluations in the background, while denying them access to the UI's streaming SSE endpoints. It also updates the job registry to preserve live error counts that cannot be derived from entities, and adds comprehensive tests. The review feedback suggests offloading synchronous disk I/O in _build_eval_runner to a worker thread to avoid blocking the event loop, and correcting comments in eval_api.py that reference the wrong background job endpoint.
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.
| baseline = await self.compute_state(params) | ||
| baseline_success = baseline.success | ||
|
|
||
| eval_runner = self._build_eval_runner(params) |
Contributor
There was a problem hiding this comment.
The _build_eval_runner method performs synchronous disk I/O operations (loading eval_config and run_config from disk). Calling it directly on the main thread blocks the event loop, which can degrade server responsiveness under load. We should offload this blocking I/O to a worker thread using asyncio.to_thread.
Suggested change
| eval_runner = self._build_eval_runner(params) | |
| eval_runner = await asyncio.to_thread(self._build_eval_runner, params) |
Comment on lines
+945
to
+946
| # SSE run endpoint for the UI only. Agents kick off evals via the | ||
| # non-streaming background job API (POST /api/jobs/evals). |
Contributor
There was a problem hiding this comment.
The comment references POST /api/jobs/evals as the background job API endpoint, but the actual endpoint added in this PR is POST /api/jobs/evals/run. We should update the comment to prevent confusion.
Suggested change
| # SSE run endpoint for the UI only. Agents kick off evals via the | |
| # non-streaming background job API (POST /api/jobs/evals). | |
| # SSE run endpoint for the UI only. Agents kick off evals via the | |
| # non-streaming background job API (POST /api/jobs/evals/run). |
Comment on lines
+1052
to
+1053
| # SSE run endpoint for the UI only. Agents kick off evals via the | ||
| # non-streaming background job API (POST /api/jobs/evals). |
Contributor
There was a problem hiding this comment.
The comment references POST /api/jobs/evals as the background job API endpoint, but the actual endpoint added in this PR is POST /api/jobs/evals/run. We should update the comment to prevent confusion.
Suggested change
| # SSE run endpoint for the UI only. Agents kick off evals via the | |
| # non-streaming background job API (POST /api/jobs/evals). | |
| # SSE run endpoint for the UI only. Agents kick off evals via the | |
| # non-streaming background job API (POST /api/jobs/evals/run). |
📊 Coverage ReportOverall Coverage: 92% Diff: origin/main...HEAD
Summary
Line-by-lineView line-by-line diff coverageapp/desktop/studio_server/chat/auto/api.pyLines 85-93 85 helper so a quiet-window timeout can't tear down the subscription.
86 """
87 async for item in iter_with_keepalive(run.bus.subscribe(), KEEPALIVE_SECONDS):
88 if isinstance(item, KeepalivePing):
! 89 yield b": ping\n\n"
90 else:
91 yield item
92 app/desktop/studio_server/chat/auto/events.pyLines 13-21 13
14 from .models import AutoRunStatus
15
16 if TYPE_CHECKING:
! 17 from .registry import AutoChatRun
18
19 __all__ = [
20 "AutoChatEventBus",
21 "KEEPALIVE_PING",app/desktop/studio_server/chat/auto/registry.pyLines 52-65 52 if explicit is not None:
53 return explicit
54 raw = os.environ.get(MAX_CONCURRENT_ENV_VAR)
55 if raw:
! 56 try:
! 57 value = int(raw)
! 58 if value > 0:
! 59 return value
! 60 except ValueError:
! 61 pass
62 return DEFAULT_MAX_CONCURRENT
63
64
65 def _extract_trace_id(payload: bytes) -> str | None:Lines 77-97 77 if KILN_SSE_CHAT_TRACE.encode() not in payload:
78 return None
79 for line in payload.split(b"\n"):
80 if not line.startswith(b"data: "):
! 81 continue
82 body = line[6:].strip()
83 if not body or body == b"[DONE]":
! 84 continue
85 try:
86 event = json.loads(body)
! 87 except (json.JSONDecodeError, TypeError):
! 88 continue
89 if isinstance(event, dict) and event.get("type") == KILN_SSE_CHAT_TRACE:
90 tid = event.get("trace_id")
91 if isinstance(tid, str) and tid:
92 return tid
! 93 return None
94
95
96 class AutoChatRun:
97 """Live in-memory machinery for one conversation's auto mode.Lines 247-255 247 if run_id is None:
248 return None
249 run = self._runs.get(run_id)
250 if run is None:
! 251 return None
252 # A run is reachable via the trace index only once it has a leaf (set at
253 # start for a trace-seeded run, or by _on_trace on the first
254 # kiln_chat_trace), so current_trace_id is non-None here. A no-trace seed
255 # (Revision R2) isn't indexed until its first trace arrives, so it can'tLines 255-263 255 # (Revision R2) isn't indexed until its first trace arrives, so it can't
256 # be resolved before then — guard defensively for the type checker.
257 current_trace_id = run.record.current_trace_id
258 if current_trace_id is None:
! 259 return None
260 return (run_id, current_trace_id, run.record.status)
261
262 # -- start ---------------------------------------------------------------Lines 332-340 332
333 def _fresh_run_id(self) -> str:
334 run_id = _new_run_id()
335 while run_id in self._runs:
! 336 run_id = _new_run_id()
337 return run_id
338
339 # -- inbound message injection (Revision R1) -----------------------------Lines 383-391 383 directly. Publishes auto-mode-off(user_disabled). Returns False if the
384 run is unknown."""
385 run = self._runs.get(run_id)
386 if run is None:
! 387 return False
388
389 # Mark USER_DISABLED first so a cancelled burst's CancelledError handler
390 # preserves it (rather than forcing USER_STOPPED) and publishes the
391 # correct off-reason.Lines 389-397 389 # Mark USER_DISABLED first so a cancelled burst's CancelledError handler
390 # preserves it (rather than forcing USER_STOPPED) and publishes the
391 # correct off-reason.
392 if run.record.status.is_terminal:
! 393 return True
394 run.record.status = AutoRunStatus.USER_DISABLED
395
396 task = self._tasks.get(run_id)
397 if task is not None:Lines 399-408 399 try:
400 await task
401 except asyncio.CancelledError:
402 pass
! 403 except Exception:
! 404 logger.debug(
405 "Auto run %s raised during disable await", run_id, exc_info=True
406 )
407 return TrueLines 439-457 439 # (RUNNING/IDLE) → USER_STOPPED. A cancel from disable() has
440 # already set USER_DISABLED; preserve any already-off status so
441 # we publish the correct off-reason (CR Moderate 2).
442 if not run.record.status.is_terminal:
! 443 run.record.status = AutoRunStatus.USER_STOPPED
444 run.inbound.clear()
445 self._publish_off(run)
446 self._touch(run)
447 raise
! 448 except Exception:
449 # An unrecoverable burst error ends the burst but leaves the flag
450 # on so the user can retry or stop (functional spec §4.4).
! 451 logger.exception("Auto run %s burst failed", run_id)
! 452 run.runner.idle_reason = "error"
! 453 run.record.status = AutoRunStatus.IDLE
454 else:
455 # If the flag was already cleared off-band (e.g. disable() raced
456 # in just as the burst returned), don't resurrect it — leave the
457 # terminal status as-is. Otherwise adopt the runner's status.Lines 461-470 461 if run.record.status.is_terminal:
462 # Explicitly disabled (or otherwise off): publish off, GC handled
463 # in finally. Do NOT settle to IDLE (that would re-enable the
464 # flag and republish the idle marker — CR Moderate 2).
! 465 run.inbound.clear()
! 466 self._publish_off(run)
467 else:
468 # Settled burst → IDLE; the flag stays on, entry not evicted.
469 run.record.status = AutoRunStatus.IDLE
470 run.bus.publish(run.idle_marker_bytes())Lines 490-498 490
491 def _on_trace(self, run_id: str, new_trace_id: str) -> None:
492 run = self._runs.get(run_id)
493 if run is None:
! 494 return
495 if new_trace_id not in run.record.seen_trace_ids:
496 run.record.seen_trace_ids.append(new_trace_id)
497 run.record.current_trace_id = new_trace_id
498 self._trace_index[new_trace_id] = run_idLines 529-538 529 try:
530 await task
531 except asyncio.CancelledError:
532 pass
! 533 except Exception:
! 534 logger.debug(
535 "Auto run %s raised during stop await", run_id, exc_info=True
536 )
537 returnLines 546-554 546
547 def _schedule_gc(self, run_id: str) -> None:
548 existing = self._gc_tasks.get(run_id)
549 if existing is not None and not existing.done():
! 550 return
551 self._gc_tasks[run_id] = asyncio.create_task(self._gc_after_ttl(run_id))
552
553 def _cancel_gc(self, run_id: str) -> None:
554 """Defensive: a fresh burst should never start on a GC-scheduled runLines 555-563 555 (only off runs are GC'd, and a new burst only starts from IDLE), but
556 cancel any pending GC if one exists."""
557 gc_task = self._gc_tasks.pop(run_id, None)
558 if gc_task is not None:
! 559 gc_task.cancel()
560
561 async def _gc_after_ttl(self, run_id: str) -> None:
562 try:
563 await asyncio.sleep(TERMINAL_TTL_SECONDS)Lines 569-577 569
570 def _evict(self, run_id: str) -> None:
571 run = self._runs.pop(run_id, None)
572 if run is None:
! 573 return
574 for tid in list(self._trace_index):
575 if self._trace_index[tid] == run_id:
576 del self._trace_index[tid]
577 logger.debug("Evicted terminal auto run %s after TTL", run_id)app/desktop/studio_server/chat/auto/runner.pyLines 152-160 152
153 def request_stop(self) -> None:
154 """Mark a graceful stop. The current round finishes; the loop then ends
155 at the next boundary (no cancel, no cut-off)."""
! 156 self.stop_requested = True
157
158 async def run(self) -> None:
159 self._emit(format_auto_mode_on(self.run_id))
160 body = await self._build_seed_body()Lines 287-301 287 # No client tool results to feed back (e.g. server-only tool
288 # batch with no client tools to approve). On graceful stop just
289 # disable — there's nothing to surface for approval.
290 if self.stop_requested:
! 291 self.status = AutoRunStatus.USER_STOPPED
! 292 return
293 # Same drain-before-idle check applies.
294 injected = self._drain()
295 if injected:
! 296 body = self._continue_with_user_messages(body, injected)
! 297 continue
298 self.idle_reason = "done"
299 self.status = AutoRunStatus.IDLE
300 returnLines 308-317 308 # Graceful stop after a fully auto-executed round: don't start a new
309 # round. The tool results were fed back so the trace stays clean;
310 # disable auto mode (the supervisor publishes auto-mode-off).
311 if self.stop_requested:
! 312 self.status = AutoRunStatus.USER_STOPPED
! 313 return
314 # Inject any messages queued during this round alongside the tool
315 # results so the backend sees both on the next turn (§13.2).
316 injected = self._drain()
317 if injected:Lines 434-442 434 # runner is only ever fed via that enqueue path. Echoing again here would
435 # render the injected message twice to all observers, so drain only takes
436 # the queued messages and does NOT re-echo (CR Moderate 1).
437 if self._drain_inbound is None:
! 438 return []
439 return self._drain_inbound()
440
441 @staticmethod
442 def _append_user_messages(app/desktop/studio_server/chat/routes.pyLines 266-274 266 if detailed.status_code == HTTPStatus.OK and isinstance(detailed.parsed, list):
267 items: list[ChatSessionListItem] = []
268 for item in detailed.parsed:
269 if not isinstance(item, ApiSessionListItem):
! 270 continue
271 # Server-side join against the in-memory auto-run registry so the
272 # UI gets a single, point-in-time view of which sessions are
273 # actively running in auto mode (no two-list correlation client
274 # side). A sub-ms race here is self-healing on the next refresh.app/desktop/studio_server/chat/stream_session.pyLines 253-262 253 try:
254 parsed = json.loads(error_body)
255 detail = parsed.get("message", detail) or detail
256 code = parsed.get("code")
! 257 except json.JSONDecodeError:
! 258 pass
259 error_payload: dict[str, Any] = {
260 "type": "error",
261 "message": detail,
262 }Lines 278-287 278 round_state.error_retryable = (
279 upstream.status_code in RETRYABLE_UPSTREAM_STATUS
280 )
281 return
! 282 yield error_bytes
! 283 return
284
285 try:
286 async for chunk in upstream.aiter_bytes():
287 result = parser.parse(chunk)Lines 323-331 323 # already dropped mid-round (possibly after partial content),
324 # so re-POSTing is not safe to do blindly.
325 round_state.deferred_error_payload = error_bytes
326 else:
! 327 yield error_bytes
328 logger.exception(
329 "RemoteProtocolError during streaming (trace_id=%s)",
330 trace_id,
331 )Lines 428-437 428 continue
429
430 # Stop takes precedence over surfacing the error (auto runner only).
431 if stop:
! 432 result.status = "stopped"
! 433 return
434 # Give up: surface the held-back error (the suppressed-duplicate terminal
435 # case has nothing deferred and yields nothing) and end the stream.
436 if round_state.deferred_error_payload is not None:
437 yield round_state.deferred_error_payloadLines 662-670 662 """Clear the conversation's auto-mode flag for an intercepted
663 disable_auto_mode call. Imported lazily to avoid a circular import
664 (the auto registry depends on this module's round mechanics)."""
665 if not trace_id:
! 666 return
667 from app.desktop.studio_server.chat.auto.registry import auto_chat_registry
668
669 await auto_chat_registry.disable_for_trace(trace_id)app/desktop/studio_server/jobs/api.pyLines 210-218 210 type_name=JudgeFeedbackBatchJobWorker.type_name,
211 params=params,
212 project_id=params.project_id,
213 )
! 214 return CreateJobResponse(job_id=job.id, status=job.status)
215
216 @app.post(
217 "/api/jobs/wait",
218 summary="Wait For Jobs",app/desktop/studio_server/jobs/events.pyLines 77-85 77 except asyncio.TimeoutError:
78 # Cancels only the throwaway get() above; the feeder (and thus
79 # the subscription) is untouched and keeps draining.
80 yield KEEPALIVE_PING
! 81 continue
82 if item is None:
83 break
84 yield item
85 finally:app/desktop/studio_server/jobs/workers/eval.pyLines 362-371 362
363 def _eval_and_task(self, eval_config: EvalConfig) -> tuple[Eval, Task]:
364 eval = eval_config.parent_eval()
365 if eval is None:
! 366 raise ValueError("Eval config has no parent eval")
367 task = eval.parent_task()
368 if task is None:
! 369 raise ValueError("Eval has no parent task")
370 return eval, tasklibs/core/kiln_ai/adapters/eval/judge_feedback_batch_runner.pyLines 58-66 58 (often generic) `format_error_message` text; the underlying cause survives on `.original`, so
59 surface that for the developer-facing error log. Mirrors `EvalJobWorker._error_detail`.
60 """
61 if isinstance(error, KilnRunError) and error.original is not None:
! 62 return str(error.original)
63 return str(error)
64
65
66 def score_passes(Lines 80-88 80 ) -> bool:
81 """An example 'fails' only if ALL of the eval's (non-custom) output scores are below the bar."""
82 relevant = [s for s in output_scores if s.type != TaskOutputRatingType.custom]
83 if not relevant:
! 84 return False
85 for score in relevant:
86 value = scores.get(score.json_key())
87 # A missing score can't be confirmed as a pass, so treat it as failing and keep checking.
88 if value is not None and score_passes(value, score.type, threshold):Lines 101-109 101 """
102 aggregated: dict[str, float] = {}
103 for score in output_scores:
104 if score.type == TaskOutputRatingType.custom:
! 105 continue
106 key = score.json_key()
107 values: list[float] = []
108 for run in runs:
109 value = run.scores.get(key)Lines 107-119 107 values: list[float] = []
108 for run in runs:
109 value = run.scores.get(key)
110 if value is None:
! 111 continue
112 try:
113 values.append(normalize_rating(value, score.type))
! 114 except (ValueError, TypeError):
! 115 continue
116 if values:
117 aggregated[key] = sum(values) / len(values)
118 return aggregatedLines 232-240 232 retry_delay: float = 2.0,
233 ):
234 task = judge_feedback_batch.parent_task()
235 if task is None:
! 236 raise ValueError("Judge feedback batch must have a parent task")
237 eval = eval_config.parent_eval()
238 if eval is None:
239 raise ValueError("Eval config must have a parent eval")Lines 272-280 272 ),
273 None,
274 )
275 if run_config is None:
! 276 raise ValueError(
277 f"Run config not found for generate mode: {judge_feedback_batch.run_config_id}"
278 )
279 self._run_config_properties = run_config.run_config_properties
280 self._skills = load_skills_for_task(task, run_config.run_config_properties)Lines 312-320 312 stop_after = job.stop_after_failures
313 # Generating runs a model per item (heavier than judging an existing output), so default to
314 # a smaller concurrency in that mode.
315 if concurrency is None:
! 316 concurrency = 5 if job.generate_outputs else 25
317 # A caller-supplied concurrency of 0 would make range() raise; negatives would mis-chunk.
318 concurrency = max(1, concurrency)
319
320 candidates = [Lines 344-352 344 evaluator = eval_adapter_from_type(self.eval_config.config_type)(
345 self.eval_config, self._run_config_properties, skills=self._skills
346 )
347 if not isinstance(evaluator, BaseEval):
! 348 raise ValueError("Not able to create evaluator from eval config")
349
350 failing_runs: list[JudgeFeedbackBatchRun] = []
351 judged_runs: list[JudgeFeedbackBatchRun] = []
352 errors: list[JudgeFeedbackBatchItemError] = []Lines 377-385 377 error=f"Unexpected error judging item: {_error_detail(scored)}",
378 )
379 errors.append(unexpected_error)
380 if error_callback is not None:
! 381 await error_callback(unexpected_error)
382 continue
383 if scored.error is not None:
384 errors.append(scored.error)
385 if error_callback is not None:libs/core/kiln_ai/datamodel/judge_feedback_batch.pyLines 12-20 12 from kiln_ai.datamodel.eval import EvalScores
13 from kiln_ai.datamodel.usage import Usage
14
15 if TYPE_CHECKING:
! 16 from kiln_ai.datamodel.task import Task
17
18
19 class JudgeFeedbackBatchRun(KilnParentedModel):
20 """The judge's result for a single sampled dataset item (a child of a JudgeFeedbackBatch)."""Lines 48-56 48 if (
49 self.parent is not None
50 and self.parent.__class__.__name__ != "JudgeFeedbackBatch"
51 ):
! 52 raise ValueError("parent must be a JudgeFeedbackBatch")
53 return self.parent # type: ignore
54
55
56 class JudgeFeedbackBatch(Lines 123-132 123 return self
124
125 def parent_task(self) -> Union["Task", None]:
126 if self.parent is not None and self.parent.__class__.__name__ != "Task":
! 127 raise ValueError("parent must be a Task")
128 return self.parent # type: ignore
129
130 def runs(self, readonly: bool = False) -> list[JudgeFeedbackBatchRun]:
! 131 return super().runs(readonly=readonly) # type: ignorelibs/core/kiln_ai/datamodel/task.pyLines 256-264 256
257 def judge_feedback_batches(
258 self, readonly: bool = False
259 ) -> list[JudgeFeedbackBatch]:
! 260 return super().judge_feedback_batches(readonly=readonly) # type: ignore
261
262 # Workaround to return typed parent without importing Task
263 def parent_project(self) -> Union["Project", None]:
264 if self.parent is None or self.parent.__class__.__name__ != "Project":
|
![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: 3
🤖 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/eval_api.py`:
- Around line 945-947: The comment text in the eval API route definitions uses
the wrong background job endpoint; update the references in the SSE run endpoint
and related comment block to point to POST /api/jobs/evals/run instead of POST
/api/jobs/evals. Use the surrounding eval route definitions in eval_api.py,
especially the SSE run endpoint and the background job API comment near
openapi_extra=DENY_AGENT, to locate and correct both occurrences.
In `@app/desktop/studio_server/jobs/test_registry.py`:
- Around line 658-677: This test only covers pause() and _apply_derived(), so it
does not verify that the preserved error count survives a later resume()
progress update. Extend
test_apply_derived_preserves_error_when_compute_state_returns_none by resuming
the same job after pause() and asserting the progress.error value remains 2
after the resumed progress path in JobRegistry/EvalJobWorker.run(). Keep the
existing assertions and add the resume-path check to lock in the intended
contract.
In `@app/desktop/studio_server/jobs/workers/eval.py`:
- Around line 95-115: The resume path in eval job progress handling is resetting
error counts instead of preserving the existing baseline, so prior errors get
overwritten. Update the `run()` flow in `EvalJob` to seed `error` from the job’s
current stored error count (or pass an error baseline in alongside
`compute_state()`), then accumulate `progress.errors` before calling
`ctx.report_progress()` and returning `EvalJobResult`, so the reported error
total stays absolute across resume/reconcile.
🪄 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: d146fee8-22b8-4a8d-abd8-a4a1f392babb
📒 Files selected for processing (12)
app/desktop/studio_server/eval_api.pyapp/desktop/studio_server/jobs/api.pyapp/desktop/studio_server/jobs/models.pyapp/desktop/studio_server/jobs/registry.pyapp/desktop/studio_server/jobs/test_api.pyapp/desktop/studio_server/jobs/test_registry.pyapp/desktop/studio_server/jobs/workers/eval.pyapp/desktop/studio_server/jobs/workers/test_eval.pyapp/web_ui/src/lib/api_schema.d.tslibs/server/kiln_server/utils/agent_checks/annotations/get_api_projects_project_id_tasks_task_id_evals_eval_id_eval_config_eval_config_id_run_comparison.jsonlibs/server/kiln_server/utils/agent_checks/annotations/get_api_projects_project_id_tasks_task_id_evals_eval_id_run_calibration.jsonlibs/server/kiln_server/utils/agent_checks/annotations/post_api_jobs_evals_run.json
Comment on lines
+945
to
+947
| # SSE run endpoint for the UI only. Agents kick off evals via the | ||
| # non-streaming background job API (POST /api/jobs/evals). | ||
| openapi_extra=DENY_AGENT, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the background job route in the comments.
The PR contract names the agent kickoff endpoint as POST /api/jobs/evals/run, but both comments point to POST /api/jobs/evals.
Suggested fix
- # non-streaming background job API (POST /api/jobs/evals).
+ # non-streaming background job API (POST /api/jobs/evals/run).Also applies to: 1052-1054
🤖 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/eval_api.py` around lines 945 - 947, The comment
text in the eval API route definitions uses the wrong background job endpoint;
update the references in the SSE run endpoint and related comment block to point
to POST /api/jobs/evals/run instead of POST /api/jobs/evals. Use the surrounding
eval route definitions in eval_api.py, especially the SSE run endpoint and the
background job API comment near openapi_extra=DENY_AGENT, to locate and correct
both occurrences.
Comment on lines
+658
to
+677
| @pytest.mark.asyncio | ||
| async def test_apply_derived_preserves_error_when_compute_state_returns_none(): | ||
| # A compute_state that returns error=None (errors not derivable from | ||
| # entities) must not wipe a live error count reported via report_progress. | ||
| # error=None means "unknown, keep what we had", mirroring total/message. | ||
| reg = JobRegistry(max_concurrent=2) | ||
| reg.register_type(ErrorThenNoneWorker) | ||
| ErrorThenNoneWorker.started = asyncio.Event() | ||
| ErrorThenNoneWorker.gate = asyncio.Event() | ||
| job = await reg.create("error_then_none", {}) | ||
| await wait_for_status(reg, job.id, BackgroundJobStatus.RUNNING) | ||
| await asyncio.wait_for(ErrorThenNoneWorker.started.wait(), timeout=3.0) | ||
| assert reg._jobs[job.id].progress.error == 2 | ||
|
|
||
| # pause() runs compute_state (error=None, success=3) through _apply_derived; | ||
| # the prior error count of 2 must survive while success advances. | ||
| result = await reg.pause(job.id) | ||
| assert result.status == BackgroundJobStatus.PAUSED | ||
| assert result.progress.error == 2 | ||
| assert result.progress.success == 3 |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a resume-path assertion here.
This only exercises pause()/_apply_derived(). The live error count can still regress on the next resume() progress update, so this test will not catch the multi-attempt undercount in EvalJobWorker.run(). Extending it through resume() would lock in the intended contract.
🤖 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/jobs/test_registry.py` around lines 658 - 677, This
test only covers pause() and _apply_derived(), so it does not verify that the
preserved error count survives a later resume() progress update. Extend
test_apply_derived_preserves_error_when_compute_state_returns_none by resuming
the same job after pause() and asserting the progress.error value remains 2
after the resumed progress path in JobRegistry/EvalJobWorker.run(). Keep the
existing assertions and add the resume-path check to lock in the intended
contract.
Comment on lines
+95
to
+115
| baseline = await self.compute_state(params) | ||
| baseline_success = baseline.success | ||
|
|
||
| eval_runner = self._build_eval_runner(params) | ||
|
|
||
| success = baseline_success | ||
| total = baseline.total if baseline.total is not None else baseline_success | ||
| error = 0 | ||
| async for progress in eval_runner.run(): | ||
| # progress.total = full - baseline_success (the unfinished remainder), | ||
| # so baseline_success + progress.total = the full eval-set size. | ||
| success = baseline_success + progress.complete | ||
| total = baseline_success + progress.total | ||
| error = progress.errors | ||
| await ctx.report_progress( | ||
| success=success, | ||
| error=error, | ||
| total=total, | ||
| ) | ||
|
|
||
| return EvalJobResult(total=total, success=success, error=error) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Carry forward prior error counts on resume.
compute_state() now intentionally leaves error=None so the registry can preserve the live count across reconcile, but run() still restarts from error = 0 and forwards raw progress.errors. Because JobContext.report_progress() stores absolute counts, the first progress event after a pause/resume overwrites any earlier errors, and EvalJobResult.error only reflects the latest attempt. Seed this mapping from the job’s existing error count (or have the registry provide that baseline) before translating runner progress.
🤖 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/jobs/workers/eval.py` around lines 95 - 115, The
resume path in eval job progress handling is resetting error counts instead of
preserving the existing baseline, so prior errors get overwritten. Update the
`run()` flow in `EvalJob` to seed `error` from the job’s current stored error
count (or pass an error baseline in alongside `compute_state()`), then
accumulate `progress.errors` before calling `ctx.report_progress()` and
returning `EvalJobResult`, so the reported error total stays absolute across
resume/reconcile.
Salvages the assistant auto-mode prototype (PR #1451) onto leonard/kil-686-eval-job, dropping the eval/finetune/RAG-via-job refactor (PRs #1436/#1450) that the original branch was stacked on. The auto-mode feature is self-contained: a new chat/auto/ app-server subsystem (auto-run registry/runner/events/SSE + API), enable/disable auto-mode built-in tools in libs/core, and the assistant web UI (auto_run_store, consent dialog, chat history, chat.svelte). Its only coupling to the dropped stack was a generic SSE keepalive helper, which the dropped stack had extracted into jobs/events.py; that helper (KeepalivePing / KEEPALIVE_PING / iter_with_keepalive) is ported here so chat/auto reuses it unchanged. Conflict resolutions vs the newer base: - chat.svelte: took auto-mode's version (the built+tested feature) and re-applied the base's three UI refactors landed since the fork — scrollbar-to-the-side, DaisyUI btn-circle send/stop, and the centering wrapper div (content centered while the scrollbar stays at the edge). - .env.example: kept PUBLIC_ENABLE_JOBS; dropped PUBLIC_SHOW_TOOL_CALL_DETAILS (auto-mode removed that debug feature). Regenerated agent-check annotations and the OpenAPI TS schema. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013QNY986xT2VCm7xngUg4tF
Low-risk quick-wins from automated review: - jobs/events.py: make iter_with_keepalive generic (TypeVar) so the bytes SSE consumer in chat/auto is typed correctly, not just the JobEvent case. - chat/auto/models.py: pin InboundMessage.role to Literal["user"] so the /message endpoint can't be handed a system/assistant role. - auto_run_store.ts: clear the optimistic working flag when an inject send fails (no burst started, so nothing else would clear it). - chat_session_store.ts: in handleAutoModeConsent, fall back to the last assistant message's trace before continuationTraceId so live-chat consent events with a null payload trace aren't dropped. - chat.svelte: hold consentPending through requestEnable() so a slow enable can't re-enable the button and double-dispatch. - chat_history_row.svelte: reveal the delete action on keyboard focus (group-focus-within), not just hover. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013QNY986xT2VCm7xngUg4tF
If the user switches conversations while resyncOnLoad's resolve() or snapshot GET is in flight, the resolved stale run could be hydrated into and attached onto the newly-selected session. Re-check the active trace after each await and bail with a plain return (never detach()/loadSession, since the shared auto_run_store may already be owned by the new session). Addresses the resync race flagged in PR review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013QNY986xT2VCm7xngUg4tF
Two fixes to the background-job/eval flow:
- Eval error log: EvalRunner.run() now accepts observers, and EvalJobWorker
passes one that forwards each failed dataset item's exception to
ctx.report_error. Previously only the error COUNT (Progress.errors) was
reported, so GET /api/jobs/{id}/errors showed "no errors recorded" even
when every item failed.
- Multi-job wait: add GET /api/jobs/wait?ids=a&ids=b&timeout=, backed by
JobRegistry.wait_many(), to block until ALL given jobs are terminal and
return their records. Lets a caller that kicked off several eval jobs (one
per run config) wait for them in one call. Declared before /api/jobs/{id}
so "wait" doesn't resolve to {id}. Pure observer, like /{id}/wait.
Regenerated the OpenAPI TS schema and agent-check annotations.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013QNY986xT2VCm7xngUg4tF
leonardmq
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 13
🧹 Nitpick comments (1)
app/desktop/studio_server/jobs/events.py (1)
50-53: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse a dedicated end-of-stream sentinel.
iter_with_keepalive()is generic over_T, but the feeder reservesNoneas an internal terminator. Any subscription that legitimately yieldsNonewill be treated as EOF and cut off early. A private sentinel object keeps the helper's advertised contract intact.♻️ Suggested change
+_END = object() + async def iter_with_keepalive( subscription: AsyncGenerator[_T, None], timeout_seconds: float, ) -> AsyncGenerator[_T | KeepalivePing, None]: @@ - local_queue: asyncio.Queue[_T | None] = asyncio.Queue() + local_queue: asyncio.Queue[_T | object] = asyncio.Queue() @@ finally: # End-of-stream sentinel. Also reached if the feeder is cancelled # during teardown — harmless, since the consumer is gone by then. - local_queue.put_nowait(None) + local_queue.put_nowait(_END) @@ - if item is None: + if item is _END: breakAlso applies to: 60-62, 76-77
🤖 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/jobs/events.py` around lines 50 - 53, `iter_with_keepalive()` currently uses `None` as the internal end-of-stream marker, which breaks legitimate `_T` values that can be `None`. Update the helper to use a private dedicated sentinel object instead of `None`, and adjust the `local_queue`/feeder/consumer checks in `iter_with_keepalive()` so only that sentinel ends the loop. Keep the change localized to the generator and its teardown path so the generic contract remains intact.
🤖 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/chat/auto/events.py`:
- Around line 61-83: Avoid double-emitting replayed control markers on reattach:
`AutoChatRun.emit()` already replays `self._run.buffer`, so `AutoChatRun` in
`app/desktop/studio_server/chat/auto/events.py` should not synthesize
`terminal_off_bytes()`, `idle_marker_bytes()`, or `state_marker_bytes()` again
if the matching marker is already present in the replayed buffer. Update the
reattach logic to inspect the buffered payloads (or only inject when the replay
is empty) before yielding any synthetic marker, especially for the
`AutoRunStatus.IDLE` and terminal paths.
In `@app/desktop/studio_server/chat/auto/registry.py`:
- Around line 360-362: The AutoChatSeed setup in registry.py is picking the
trace from message.trace_id first, which can resume an idle burst from an older
branch instead of the registry’s current leaf. Update the seed construction in
the registry flow so it uses run.record.current_trace_id as the authoritative
trace for resuming idle bursts, and only falls back to message.trace_id if the
current trace is unavailable. Keep the change localized around AutoChatSeed and
the surrounding resume logic so stale tabs cannot fork or regress the
conversation.
In `@app/desktop/studio_server/chat/auto/runner.py`:
- Around line 251-256: The late-stop path in `AutoRunner` is returning too early
after `execute_tool_batch()` and skips publishing the executed tool result
`body` upstream, so the trace remains incomplete. In the `run` flow around the
`self.stop_requested` check, make sure any locally emitted tool outputs are
flushed/posted before setting `AutoRunStatus.USER_STOPPED` and returning. Use
the existing `execute_tool_batch()` result-handling and the upstream posting
logic in `AutoRunner` to ensure the `body` is persisted even when a stop arrives
mid-batch.
- Around line 186-192: The disable-handling block in runner.py can leave auto
mode enabled if _resolve_disable() fails before status is updated. In
AutoRunner’s disable_evt branch, set self.status to AutoRunStatus.USER_DISABLED
before awaiting _resolve_disable(), and keep disable_trace_id assignment there
so the runner is marked disabled locally even on failure. Then wrap
_resolve_disable() in error handling that logs the resolution failure without
reverting status or re-enabling auto mode.
In `@app/desktop/studio_server/chat/stream_session.py`:
- Around line 135-140: The consent SSE payload in stream_session.py is using
snake_case keys, but the web UI expects the existing camelCase event shape.
Update the payload built in the auto-mode consent path so the identifiers
exposed by the SSE event use traceId, enableToolCallId, and siblingToolCalls
consistently with the client contract, keeping the rest of the event structure
unchanged.
- Around line 327-355: Use the last known trace when handling auto-mode
interception in stream_session.py instead of only round_state.trace_id. In the
disable_auto_mode path inside the streaming logic, switch the consent SSE
emission and _clear_auto_mode_flag call to use RoundState.trace_id_for_error so
continuation rounds without a fresh trace still publish the correct trace and
successfully clear the active auto-mode state.
In `@app/desktop/studio_server/jobs/registry.py`:
- Around line 554-563: In wait_many, avoid re-reading self._jobs at the end
because a job can be deleted after validation and before the return, causing a
KeyError. Keep the validated JobRecord objects obtained via
self._require(job_id) in a local list during the initial loop, use those
references for the final return, and preserve the existing completion-event
waiting logic in wait_many and _completion_events without a terminal-state
relookup.
In `@app/desktop/studio_server/jobs/test_api.py`:
- Around line 632-634: The /api/jobs/wait test currently checks returned job IDs
as a set, so it misses order regressions even though the endpoint contract
preserves order. Update the assertion in the test around body handling to
compare the IDs as a list in the expected sequence from the request, while
keeping the existing status checks in place.
In `@app/web_ui/src/lib/chat/auto_run_store.ts`:
- Around line 408-419: The burst-state handling in auto_run_store.ts is not
being propagated into attach(), so manual enable flows with only trace_id can
briefly look working when they should be idle. Update the call site around
startsBurst/armed.set in the auto-run flow to pass the already computed
burst/liveness state into attach(data.run_id) instead of relying on attach’s
default working=true behavior, using the existing startsBurst logic in this
path.
In `@app/web_ui/src/lib/chat/chat_session_store.ts`:
- Around line 606-630: beginArmedAutoRunInner currently appends an optimistic
userMessage and updates persisted.lastSentAppState before
autoRunStore.requestEnable, but on failure it leaves that unsent turn in the
transcript. Add rollback logic in beginArmedAutoRunInner (and any helper it
uses) so that when result.ok is false, the previously appended user message is
removed from messages and persisted state is restored to its prior value before
returning false. Make sure the fix preserves the existing success path and only
reverts the optimistic turn on enable failure.
In `@app/web_ui/src/lib/chat/streaming_chat.ts`:
- Around line 701-719: resumePendingToolCalls() currently only special-cases
tool-calls-pending, so auto-mode-consent-required from the resumed execute-tools
stream is dropped instead of prompting the user. Update the event handling in
streaming_chat.ts within resumePendingToolCalls() and the StreamEventProcessor
path it uses so auto-mode-consent-required is intercepted the same way
streamChat() already does, and route it to the existing consent/prompt flow
rather than letting it fall through unhandled.
In `@app/web_ui/src/routes/`(app)/assistant/chat_history_row.svelte:
- Around line 25-30: The row-level keydown handler in the chat history row is
responding to bubbled Enter/Space events from the action menu, causing
onSelect(row) to fire when users are interacting with TableActionMenu. Update
the keydown logic in the row component so it only selects when the event
originated from the row itself by checking e.target === e.currentTarget, or
alternatively stop keydown propagation from the TableActionMenu container; apply
the same fix wherever the duplicated row/menu keydown handling appears.
In `@app/web_ui/src/routes/`(app)/assistant/chat.svelte:
- Around line 53-73: Manual auto-mode can still be triggered while a chat
response is streaming, which can cause traceIdForNextChatRequest(messages) to
arm or enable the wrong run. Update the footer button gating and/or
openManualAutoMode in chat.svelte to block invocation whenever isLoading is
true, using the existing isLoading state alongside consentPending so the enable
flow cannot start mid-turn.
---
Nitpick comments:
In `@app/desktop/studio_server/jobs/events.py`:
- Around line 50-53: `iter_with_keepalive()` currently uses `None` as the
internal end-of-stream marker, which breaks legitimate `_T` values that can be
`None`. Update the helper to use a private dedicated sentinel object instead of
`None`, and adjust the `local_queue`/feeder/consumer checks in
`iter_with_keepalive()` so only that sentinel ends the loop. Keep the change
localized to the generator and its teardown path so the generic contract remains
intact.
🪄 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: 21253ccc-7024-419a-9025-1d45f67445c7
📒 Files selected for processing (76)
app/desktop/desktop_server.pyapp/desktop/studio_server/chat/__init__.pyapp/desktop/studio_server/chat/auto/__init__.pyapp/desktop/studio_server/chat/auto/api.pyapp/desktop/studio_server/chat/auto/events.pyapp/desktop/studio_server/chat/auto/models.pyapp/desktop/studio_server/chat/auto/registry.pyapp/desktop/studio_server/chat/auto/runner.pyapp/desktop/studio_server/chat/auto/sse.pyapp/desktop/studio_server/chat/auto/test_api.pyapp/desktop/studio_server/chat/auto/test_fakes.pyapp/desktop/studio_server/chat/auto/test_iter_upstream_round.pyapp/desktop/studio_server/chat/auto/test_registry.pyapp/desktop/studio_server/chat/auto/test_runner.pyapp/desktop/studio_server/chat/constants.pyapp/desktop/studio_server/chat/routes.pyapp/desktop/studio_server/chat/stream_session.pyapp/desktop/studio_server/chat/test_routes.pyapp/desktop/studio_server/chat/test_stream_session.pyapp/desktop/studio_server/jobs/api.pyapp/desktop/studio_server/jobs/events.pyapp/desktop/studio_server/jobs/registry.pyapp/desktop/studio_server/jobs/test_api.pyapp/desktop/studio_server/jobs/test_registry.pyapp/desktop/studio_server/jobs/workers/eval.pyapp/desktop/studio_server/jobs/workers/test_eval.pyapp/web_ui/.env.exampleapp/web_ui/src/lib/api_schema.d.tsapp/web_ui/src/lib/chat/auto_run_store.test.tsapp/web_ui/src/lib/chat/auto_run_store.tsapp/web_ui/src/lib/chat/chat_session_store.test.tsapp/web_ui/src/lib/chat/chat_session_store.tsapp/web_ui/src/lib/chat/session_grouping.test.tsapp/web_ui/src/lib/chat/session_grouping.tsapp/web_ui/src/lib/chat/session_messages.test.tsapp/web_ui/src/lib/chat/session_messages.tsapp/web_ui/src/lib/chat/streaming_chat.test.tsapp/web_ui/src/lib/chat/streaming_chat.tsapp/web_ui/src/routes/(app)/assistant/+page.svelteapp/web_ui/src/routes/(app)/assistant/auto_mode_consent_dialog.svelteapp/web_ui/src/routes/(app)/assistant/auto_mode_consent_dialog.test.tsapp/web_ui/src/routes/(app)/assistant/chat.svelteapp/web_ui/src/routes/(app)/assistant/chat_history.svelteapp/web_ui/src/routes/(app)/assistant/chat_history_row.svelteapp/web_ui/src/routes/(app)/assistant/chat_step_group.sveltelibs/core/kiln_ai/adapters/eval/eval_runner.pylibs/core/kiln_ai/datamodel/tool_id.pylibs/core/kiln_ai/tools/built_in_tools/disable_auto_mode_tool.pylibs/core/kiln_ai/tools/built_in_tools/enable_auto_mode_tool.pylibs/core/kiln_ai/tools/built_in_tools/test_disable_auto_mode_tool.pylibs/core/kiln_ai/tools/built_in_tools/test_enable_auto_mode_tool.pylibs/core/kiln_ai/tools/test_tool_registry.pylibs/core/kiln_ai/tools/tool_registry.pylibs/server/kiln_server/utils/agent_checks/annotations/get_api_chat_auto_resolve.jsonlibs/server/kiln_server/utils/agent_checks/annotations/get_api_chat_auto_run_id_events.jsonlibs/server/kiln_server/utils/agent_checks/annotations/get_api_chat_auto_sessions.jsonlibs/server/kiln_server/utils/agent_checks/annotations/get_api_jobs_wait.jsonlibs/server/kiln_server/utils/agent_checks/annotations/post_api_chat_auto_decline.jsonlibs/server/kiln_server/utils/agent_checks/annotations/post_api_chat_auto_enable.jsonlibs/server/kiln_server/utils/agent_checks/annotations/post_api_chat_auto_run_id_message.jsonlibs/server/kiln_server/utils/agent_checks/annotations/post_api_chat_auto_run_id_stop.jsonspecs/projects/assistant_auto_mode/architecture.mdspecs/projects/assistant_auto_mode/functional_spec.mdspecs/projects/assistant_auto_mode/implementation_plan.mdspecs/projects/assistant_auto_mode/phase_plans/phase_1.mdspecs/projects/assistant_auto_mode/phase_plans/phase_10.mdspecs/projects/assistant_auto_mode/phase_plans/phase_2.mdspecs/projects/assistant_auto_mode/phase_plans/phase_3.mdspecs/projects/assistant_auto_mode/phase_plans/phase_4.mdspecs/projects/assistant_auto_mode/phase_plans/phase_5.mdspecs/projects/assistant_auto_mode/phase_plans/phase_6.mdspecs/projects/assistant_auto_mode/phase_plans/phase_7.mdspecs/projects/assistant_auto_mode/phase_plans/phase_8.mdspecs/projects/assistant_auto_mode/phase_plans/phase_9.mdspecs/projects/assistant_auto_mode/project_overview.mdspecs/projects/assistant_auto_mode/ui_design.md
💤 Files with no reviewable changes (3)
- app/web_ui/src/lib/chat/session_messages.ts
- app/web_ui/.env.example
- app/web_ui/src/routes/(app)/assistant/chat_step_group.svelte
✅ Files skipped from review due to trivial changes (22)
- libs/server/kiln_server/utils/agent_checks/annotations/get_api_chat_auto_sessions.json
- libs/server/kiln_server/utils/agent_checks/annotations/post_api_chat_auto_run_id_stop.json
- libs/server/kiln_server/utils/agent_checks/annotations/post_api_chat_auto_run_id_message.json
- app/web_ui/src/lib/chat/session_grouping.ts
- app/desktop/studio_server/chat/test_stream_session.py
- app/desktop/studio_server/chat/auto/init.py
- specs/projects/assistant_auto_mode/phase_plans/phase_7.md
- specs/projects/assistant_auto_mode/project_overview.md
- libs/core/kiln_ai/tools/built_in_tools/test_enable_auto_mode_tool.py
- specs/projects/assistant_auto_mode/phase_plans/phase_2.md
- specs/projects/assistant_auto_mode/phase_plans/phase_5.md
- specs/projects/assistant_auto_mode/phase_plans/phase_8.md
- specs/projects/assistant_auto_mode/phase_plans/phase_10.md
- specs/projects/assistant_auto_mode/phase_plans/phase_9.md
- specs/projects/assistant_auto_mode/implementation_plan.md
- specs/projects/assistant_auto_mode/phase_plans/phase_1.md
- specs/projects/assistant_auto_mode/phase_plans/phase_3.md
- specs/projects/assistant_auto_mode/phase_plans/phase_6.md
- specs/projects/assistant_auto_mode/functional_spec.md
- specs/projects/assistant_auto_mode/phase_plans/phase_4.md
- specs/projects/assistant_auto_mode/architecture.md
- app/web_ui/src/lib/api_schema.d.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- app/desktop/studio_server/jobs/workers/eval.py
- app/desktop/studio_server/jobs/workers/test_eval.py
Comment on lines
+61
to
+83
| for payload in list(self._run.buffer): | ||
| yield payload | ||
| status = self._run.record.status | ||
| # Already off (explicit disable/stop) → emit the terminal marker and | ||
| # stop. (A live run will publish its own auto-mode-off via the queue.) | ||
| if status.is_terminal: | ||
| yield self._run.terminal_off_bytes() | ||
| return | ||
| # Idle between bursts: the flag is still on. Emit the idle marker so a | ||
| # re-attaching observer renders the persistent indicator (working | ||
| # off → "· waiting for you"), then stay subscribed for the next burst | ||
| # (started via /message). | ||
| if status == AutoRunStatus.IDLE: | ||
| yield self._run.idle_marker_bytes() | ||
| # RUNNING: a burst is actively working. The replayed buffer may be | ||
| # empty (the model is thinking server-side between events, just after | ||
| # a snapshot cleared the buffer), so without this an attaching client | ||
| # would look idle/done until the next event lands. Emit a current- | ||
| # liveness snapshot (Phase 9) so the client shows the thinking | ||
| # indicator immediately on attach. (Re-emitting working state is | ||
| # idempotent if the buffer already carried auto-mode-on.) | ||
| elif status == AutoRunStatus.RUNNING: | ||
| yield self._run.state_marker_bytes() |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Avoid double-emitting replayed control markers on reattach.
Line 61 already replays self._run.buffer, and AutoChatRun.emit() appends each emitted marker to that buffer until the next kiln_chat_trace. On an IDLE or terminal run, Lines 66-74 then synthesize the same auto-mode-off/idle marker again, so reconnects can double-fire those control events. Only inject the synthetic state when replay was empty (the empty-buffer RUNNING case this comment block describes), or explicitly inspect the replayed tail before emitting another marker.
🐛 Suggested change
try:
# Replay the in-progress turn (everything since the last snapshot).
- for payload in list(self._run.buffer):
+ replayed = list(self._run.buffer)
+ for payload in replayed:
yield payload
status = self._run.record.status
# Already off (explicit disable/stop) → emit the terminal marker and
# stop. (A live run will publish its own auto-mode-off via the queue.)
if status.is_terminal:
- yield self._run.terminal_off_bytes()
+ if not replayed:
+ yield self._run.terminal_off_bytes()
return
@@
if status == AutoRunStatus.IDLE:
- yield self._run.idle_marker_bytes()
+ if not replayed:
+ yield self._run.idle_marker_bytes()
@@
elif status == AutoRunStatus.RUNNING:
- yield self._run.state_marker_bytes()
+ if not replayed:
+ yield self._run.state_marker_bytes()🤖 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/chat/auto/events.py` around lines 61 - 83, Avoid
double-emitting replayed control markers on reattach: `AutoChatRun.emit()`
already replays `self._run.buffer`, so `AutoChatRun` in
`app/desktop/studio_server/chat/auto/events.py` should not synthesize
`terminal_off_bytes()`, `idle_marker_bytes()`, or `state_marker_bytes()` again
if the matching marker is already present in the replayed buffer. Update the
reattach logic to inspect the buffered payloads (or only inject when the replay
is empty) before yielding any synthetic marker, especially for the
`AutoRunStatus.IDLE` and terminal paths.
Comment on lines
+360
to
+362
| seed = AutoChatSeed( | ||
| trace_id=message.trace_id or run.record.current_trace_id, | ||
| extra_messages=[message.as_chat_message()], |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Resume idle bursts from the registry’s current leaf.
Line 361 prefers message.trace_id, so a stale tab can restart an idle run from an older trace and fork/regress the conversation even though current_trace_id tracks the latest leaf.
Proposed fix
seed = AutoChatSeed(
- trace_id=message.trace_id or run.record.current_trace_id,
+ trace_id=run.record.current_trace_id or message.trace_id,
extra_messages=[message.as_chat_message()],
)📝 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
| seed = AutoChatSeed( | |
| trace_id=message.trace_id or run.record.current_trace_id, | |
| extra_messages=[message.as_chat_message()], | |
| seed = AutoChatSeed( | |
| trace_id=run.record.current_trace_id or message.trace_id, | |
| extra_messages=[message.as_chat_message()], |
🤖 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/chat/auto/registry.py` around lines 360 - 362, The
AutoChatSeed setup in registry.py is picking the trace from message.trace_id
first, which can resume an idle burst from an older branch instead of the
registry’s current leaf. Update the seed construction in the registry flow so it
uses run.record.current_trace_id as the authoritative trace for resuming idle
bursts, and only falls back to message.trace_id if the current trace is
unavailable. Keep the change localized around AutoChatSeed and the surrounding
resume logic so stale tabs cannot fork or regress the conversation.
Comment on lines
+186
to
+192
| if disable_evt is not None: | ||
| self.disable_trace_id = round_state.trace_id or trace_id_for_error | ||
| await self._resolve_disable( | ||
| client, body, round_state, disable_evt, client_events | ||
| ) | ||
| self.status = AutoRunStatus.USER_DISABLED | ||
| return |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Disable locally even if the resolving continuation fails.
If _resolve_disable() raises before Line 191, the supervisor treats it as a burst error and leaves the run IDLE/flag-on. Mark the runner disabled before the await and log resolution failures without re-enabling auto mode.
Proposed fix
if disable_evt is not None:
self.disable_trace_id = round_state.trace_id or trace_id_for_error
- await self._resolve_disable(
- client, body, round_state, disable_evt, client_events
- )
self.status = AutoRunStatus.USER_DISABLED
+ try:
+ await self._resolve_disable(
+ client, body, round_state, disable_evt, client_events
+ )
+ except Exception:
+ logger.exception("Failed to resolve disable_auto_mode")
return📝 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 disable_evt is not None: | |
| self.disable_trace_id = round_state.trace_id or trace_id_for_error | |
| await self._resolve_disable( | |
| client, body, round_state, disable_evt, client_events | |
| ) | |
| self.status = AutoRunStatus.USER_DISABLED | |
| return | |
| if disable_evt is not None: | |
| self.disable_trace_id = round_state.trace_id or trace_id_for_error | |
| self.status = AutoRunStatus.USER_DISABLED | |
| try: | |
| await self._resolve_disable( | |
| client, body, round_state, disable_evt, client_events | |
| ) | |
| except Exception: | |
| logger.exception("Failed to resolve disable_auto_mode") | |
| return |
🤖 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/chat/auto/runner.py` around lines 186 - 192, The
disable-handling block in runner.py can leave auto mode enabled if
_resolve_disable() fails before status is updated. In AutoRunner’s disable_evt
branch, set self.status to AutoRunStatus.USER_DISABLED before awaiting
_resolve_disable(), and keep disable_trace_id assignment there so the runner is
marked disabled locally even on failure. Then wrap _resolve_disable() in error
handling that logs the resolution failure without reverting status or
re-enabling auto mode.
Comment on lines
+606
to
+630
| async function beginArmedAutoRunInner(text: string): Promise<boolean> { | ||
| removeErrors() | ||
| const currentAppState = getCurrentAppState() | ||
| const header = buildContextHeader( | ||
| currentAppState, | ||
| get(persisted).lastSentAppState, | ||
| ) | ||
| const userMessage: ChatMessage = { | ||
| id: chatGenerateId(), | ||
| role: "user", | ||
| content: text, | ||
| } | ||
| updateMessages((msgs) => [...msgs, userMessage]) | ||
| persisted.update((p) => ({ ...p, lastSentAppState: currentAppState })) | ||
| const apiContent = header ? header + "\n" + text : text | ||
| const seed: EnableAutoRequest = { | ||
| extra_messages: [{ role: "user", content: apiContent }], | ||
| } | ||
| const result = await autoRunStore.requestEnable(seed) | ||
| if (!result.ok) { | ||
| pushInlineError( | ||
| `Couldn't start auto mode: ${result.error ?? "unknown error"}`, | ||
| ) | ||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Rollback the optimistic user turn when armed enable fails.
beginArmedAutoRunInner() appends the user message before /enable, but on failure returns false, so the input remains while the unsent message stays in persisted transcript. A retry can duplicate the user turn and pollute chat context.
Proposed fix
async function beginArmedAutoRunInner(text: string): Promise<boolean> {
removeErrors()
+ const beforeEnable = get(persisted)
const currentAppState = getCurrentAppState()
@@
const result = await autoRunStore.requestEnable(seed)
if (!result.ok) {
+ persisted.set(beforeEnable)
pushInlineError(
`Couldn't start auto mode: ${result.error ?? "unknown error"}`,
)📝 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 beginArmedAutoRunInner(text: string): Promise<boolean> { | |
| removeErrors() | |
| const currentAppState = getCurrentAppState() | |
| const header = buildContextHeader( | |
| currentAppState, | |
| get(persisted).lastSentAppState, | |
| ) | |
| const userMessage: ChatMessage = { | |
| id: chatGenerateId(), | |
| role: "user", | |
| content: text, | |
| } | |
| updateMessages((msgs) => [...msgs, userMessage]) | |
| persisted.update((p) => ({ ...p, lastSentAppState: currentAppState })) | |
| const apiContent = header ? header + "\n" + text : text | |
| const seed: EnableAutoRequest = { | |
| extra_messages: [{ role: "user", content: apiContent }], | |
| } | |
| const result = await autoRunStore.requestEnable(seed) | |
| if (!result.ok) { | |
| pushInlineError( | |
| `Couldn't start auto mode: ${result.error ?? "unknown error"}`, | |
| ) | |
| return false | |
| } | |
| async function beginArmedAutoRunInner(text: string): Promise<boolean> { | |
| removeErrors() | |
| const beforeEnable = get(persisted) | |
| const currentAppState = getCurrentAppState() | |
| const header = buildContextHeader( | |
| currentAppState, | |
| get(persisted).lastSentAppState, | |
| ) | |
| const userMessage: ChatMessage = { | |
| id: chatGenerateId(), | |
| role: "user", | |
| content: text, | |
| } | |
| updateMessages((msgs) => [...msgs, userMessage]) | |
| persisted.update((p) => ({ ...p, lastSentAppState: currentAppState })) | |
| const apiContent = header ? header + "\n" + text : text | |
| const seed: EnableAutoRequest = { | |
| extra_messages: [{ role: "user", content: apiContent }], | |
| } | |
| const result = await autoRunStore.requestEnable(seed) | |
| if (!result.ok) { | |
| persisted.set(beforeEnable) | |
| pushInlineError( | |
| `Couldn't start auto mode: ${result.error ?? "unknown error"}`, | |
| ) | |
| return false | |
| } |
🤖 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/chat/chat_session_store.ts` around lines 606 - 630,
beginArmedAutoRunInner currently appends an optimistic userMessage and updates
persisted.lastSentAppState before autoRunStore.requestEnable, but on failure it
leaves that unsent turn in the transcript. Add rollback logic in
beginArmedAutoRunInner (and any helper it uses) so that when result.ok is false,
the previously appended user message is removed from messages and persisted
state is restored to its prior value before returning false. Make sure the fix
preserves the existing success path and only reverts the optimistic turn on
enable failure.
Comment on lines
+701
to
+719
| export interface ResumePendingToolCallsOptions { | ||
| /** ``POST /api/chat`` base URL (execute-tools URL is derived from it). */ | ||
| apiUrl: string | ||
| /** Trace id of the conversation the surfaced tool calls belong to. */ | ||
| traceId: string | ||
| /** The tool calls surfaced for approval (from a ``tool-calls-pending`` event). */ | ||
| items: ToolCallsPendingItem[] | ||
| /** Reuses the normal approval gate: toolCallId → allowed. */ | ||
| onToolCallsPending: ( | ||
| payload: ToolCallsPendingPayload, | ||
| ) => Promise<Record<string, boolean>> | ||
| onAssistantMessage: (update: (draft: ChatMessage) => void) => void | ||
| onChatTrace?: (traceId: string) => void | ||
| onInlineError?: (message: string, traceId?: string, code?: string) => void | ||
| onToolExecutionStart?: (toolCount: number) => void | ||
| onToolExecutionEnd?: (toolCount: number) => void | ||
| onShowActivityIndicator?: (show: boolean) => void | ||
| onFinish: () => void | ||
| onError: (error: Error) => void |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Handle auto-mode-consent-required in resumePendingToolCalls().
This continuation loop only intercepts tool-calls-pending. If the resumed /execute-tools stream emits auto-mode-consent-required, it falls through to StreamEventProcessor, which has no handler for that event, so the consent request is dropped and the user never gets a prompt. streamChat() already handles that control event, so these two continuation paths now behave differently.
Also applies to: 832-846
🤖 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/chat/streaming_chat.ts` around lines 701 - 719,
resumePendingToolCalls() currently only special-cases tool-calls-pending, so
auto-mode-consent-required from the resumed execute-tools stream is dropped
instead of prompting the user. Update the event handling in streaming_chat.ts
within resumePendingToolCalls() and the StreamEventProcessor path it uses so
auto-mode-consent-required is intercepted the same way streamChat() already
does, and route it to the existing consent/prompt flow rather than letting it
fall through unhandled.
Comment on lines
+25
to
+30
| on:keydown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault() | ||
| if (!busy) onSelect(row) | ||
| } | ||
| }} |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Don't let menu key presses select the row.
The row-level keydown handler will also fire for bubbled Enter/Space events from TableActionMenu. That means keyboard users can trigger onSelect(row) while trying to open or use the action menu. Guard on e.target === e.currentTarget here, or stop keydown propagation from the menu container too.
Also applies to: 57-60
🤖 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)/assistant/chat_history_row.svelte around lines
25 - 30, The row-level keydown handler in the chat history row is responding to
bubbled Enter/Space events from the action menu, causing onSelect(row) to fire
when users are interacting with TableActionMenu. Update the keydown logic in the
row component so it only selects when the event originated from the row itself
by checking e.target === e.currentTarget, or alternatively stop keydown
propagation from the TableActionMenu container; apply the same fix wherever the
duplicated row/menu keydown handling appears.
Comment on lines
+53
to
+73
| async function openManualAutoMode() { | ||
| if (consentPending) return | ||
| consentPending = true | ||
| // Hold consentPending (the button's only disable guard) through the whole | ||
| // flow — including the awaited requestEnable() — so a slow enable can't | ||
| // re-enable the button and dispatch a duplicate enable. | ||
| try { | ||
| const accepted = await consentDialog.prompt(null) | ||
| if (!accepted) return | ||
| const traceId = traceIdForNextChatRequest(messages) | ||
| if (!traceId) { | ||
| // Brand-new conversation (Revision R2): no trace to key a server run, so | ||
| // arm client-side. The indicator turns on ("waiting for you"); the first | ||
| // message creates the run (enable seeded with that message, no trace_id). | ||
| auto_run_store.arm() | ||
| return | ||
| } | ||
| // Existing conversation: enable arms a server-owned run keyed by the trace | ||
| // id (functional spec §4.1(2)). Surface enable failures (e.g. 429) instead | ||
| // of silently swallowing them — the dialog has already closed. | ||
| const result = await auto_run_store.requestEnable({ trace_id: traceId }) |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Block manual auto-mode enable during active streaming.
The footer button stays clickable while isLoading is true. If clicked mid-turn, traceIdForNextChatRequest(messages) can use a stale/empty trace and arm or enable auto mode for the wrong conversation state.
Proposed fix
async function openManualAutoMode() {
- if (consentPending) return
+ if (consentPending || isLoading) return
@@
on:click={openManualAutoMode}
- disabled={consentPending}
+ disabled={consentPending || isLoading}Also applies to: 796-800
🤖 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)/assistant/chat.svelte around lines 53 - 73,
Manual auto-mode can still be triggered while a chat response is streaming,
which can cause traceIdForNextChatRequest(messages) to arm or enable the wrong
run. Update the footer button gating and/or openManualAutoMode in chat.svelte to
block invocation whenever isLoading is true, using the existing isLoading state
alongside consentPending so the enable flow cannot start mid-turn.
Surface the kiln_server context-usage gauge contract through the studio_server
proxy to the web UI (architecture §7):
- Add a ContextUsage Pydantic model and a context_usage field to
ChatSessionSnapshot in the GET /api/chat/sessions/{id} proxy, so a session
reopened from history renders the gauge. All ContextUsage fields are optional
so an older/partial upstream never 500s the proxy.
- Make ChatSessionSnapshot's extra="ignore" explicit (Pydantic v2 default):
unknown upstream keys (notably the server-only compacted_trace) are silently
dropped at the client boundary (functional_spec §7.3 containment). Documented
that extra="forbid" must NOT be used — it would raise/500 on a leaked key.
- SSE proxy needs no change: EventParser forwards complete lines verbatim, so
context_usage on the kiln_chat_trace event passes through untouched. Added a
passthrough test asserting this.
- Tests: round-trip context_usage; drop compacted_trace at the route and via a
direct model-config invariant; tolerate a missing context_usage.
- Regenerated api_schema.d.ts so the web UI types carry context_usage
(Phase 4 depends on it).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CNJdjd98QPux2QhReKZY9z
Surface the kiln_server context_usage on the /assistant chat so users get a
glanceable, approximate signal of how full the conversation's context window is.
- streaming_chat.ts: parse context_usage off the kiln_chat_trace snapshot event
(normalizeContextUsage tolerates partial/missing upstream fields) and fire a
new onContextUsage callback alongside onChatTrace.
- chat_session_store.ts: contextUsage in PersistedChatSession (persisted to
sessionStorage), setContextUsage setter wired into the interactive stream, the
auto-run sink, and the resume/handoff path; set on history/resync load; cleared
on reset.
- session_messages.ts: hydrateSessionFromSnapshot returns contextUsage from the
session GET response (threaded through the history apply event into loadSession).
- auto_run_store.ts: AutoRunChatSink gains onContextUsage so the gauge updates
during auto-mode bursts too.
- context_usage_gauge.svelte: compact grey two-div bar (bg-base-content/10 track,
bg-base-content/30 fill, no color ramp) with the percent stacked above and a
tooltip carrying approximate {used}/{total} token counts; hidden when usage is
null. Mounted in the chat input footer row, right-aligned opposite Auto mode.
Tests: streaming_chat (parse + onContextUsage), chat_session_store (set/persist/
load/reset), session_messages (hydrate), auto_run_store sink, and a gauge
component test (markup, grey classes, fill width, percent-above, tooltip tokens,
hidden-when-null).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CNJdjd98QPux2QhReKZY9z
…ence (Phase 5) Surface the server's pre-inference compaction window in the assistant UI by handling the new kiln_compaction_status SSE event (architecture.md §8.5, functional_spec.md §9.1). - streaming_chat.ts: parse kiln_compaction_status; add onCompactionStatus through StreamEventProcessor + the interactive/resume option surfaces. Set compacting on "started"; deliberately do NOT clear on "finished" (a fast or buffered started→finished pair would collapse the window) — the indicator is cleared by the first REAL assistant content (text/tool/exec-start/snapshot) and on error. - chat_session_store.ts: runtime-only compacting flag (not persisted) wired through the interactive, resume, and auto-run sink paths; cleared on start/finish/error/reset/loadSession/new-turn/idle/off. - auto_run_store.ts: onCompactionStatus on the sink + processor. - chat.svelte / chat_status_steps.svelte: render the SAME Thinking activity indicator markup with the summarizing label. Because compaction happens before any assistant message exists, render it as a standalone activity row keyed only on compacting (a per-message mount has no DOM anchor in that window), and suppress the empty-message Thinking cursor while compacting. - studio_server: test that kiln_compaction_status passes through the raw SSE proxy untouched (no model change). - Tests: processor set/clear behavior, store flag plumbing + non-persistence, the indicator copy, and an integration test rendering chat.svelte that the summarizing row is visible with compacting=true and NO assistant message. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CNJdjd98QPux2QhReKZY9z
Addresses Gemini review feedback on the context-usage gauge tooltip: - showTooltip awaits tick() before computePosition, so Floating UI measures the tooltip's real size instead of 0×0 (it's display:none until isVisible flips), fixing wrong initial placement. - Clears any prior autoUpdate registration before re-registering and bails if the tooltip was hidden during the await, avoiding a listener leak / re-init-on-hidden race. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CNJdjd98QPux2QhReKZY9z
…ack) Addresses CodeRabbit feedback: the token-count tooltip was reachable only via mouse. Make the meter trigger focusable (tabindex=0) and show/ hide the tooltip on focus/blur as well as mouseenter/mouseleave, so keyboard users get the same info. Also clears the a11y mouse-events warning on this element. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CNJdjd98QPux2QhReKZY9z
5 tasks
sfierro
approved these changes
Jun 27, 2026
…pacting indicator
…ction KIL-727: assistant context-usage gauge + compaction indicator (app side)
leonardmq
changed the title
leonardmq
changed the title
leonardmq
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/web_ui/src/lib/chat/session_messages.ts (1)
47-58: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDropping
reasoning_contentregresses restored transcripts.With the reasoning branch removed here, assistant snapshot entries that carry only
reasoning_contentnow produce zero parts and get discarded by the laterparts.length === 0check. For mixed turns, the visible reasoning section disappears after reload/history restore, so hydrated sessions no longer match the live transcript.🤖 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/chat/session_messages.ts` around lines 47 - 58, The assistant snapshot handling in buildAssistantParts is dropping reasoning-only content, which causes restored transcript entries to disappear or lose the visible reasoning section. Update buildAssistantParts (and any related content extraction in session_messages.ts) to preserve msg.reasoning_content as a chat part alongside text and tool calls, so parts are not empty for reasoning-only assistant messages and hydrated sessions match the live transcript.
🤖 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/chat/auto/runner.py`:
- Around line 232-237: The sibling filtering in runner.py is inconsistent: the
`execute_tool_batch` path excludes `enable_auto_mode`, but `_resolve_disable()`
still auto-executes all sibling events and can reintroduce the “Unknown tool
name” case. Update `_resolve_disable()` to apply the same
`ENABLE_AUTO_MODE_TOOL_NAME` filtering used in the batch split before resolving
siblings, so mixed `disable_auto_mode` and `enable_auto_mode` batches only
process the executable events.
In `@app/web_ui/src/lib/ui/context_usage_gauge.svelte`:
- Around line 18-21: The context usage gauge only caps the upper bound, so a
negative `context_percent` can still produce invalid `aria-valuenow` and width
values. Update the `displayPercent` calculation in `context_usage_gauge.svelte`
to clamp the normalized value from `normalizeContextUsage()` to the full [0,
100] range before it is used for styling and accessibility attributes, and apply
the same fix anywhere the same calculation is duplicated.
- Around line 101-109: The focusable meter in context_usage_gauge.svelte is not
associated with its tooltip, so assistive tech only reads the short aria-label.
Add a stable id to the element rendered with role="tooltip" and reference it
from the meter trigger using aria-describedby on the bind:this={triggerElement}
element, ensuring the hover/focus tooltip text is exposed to screen readers.
Update both the meter markup and the tooltip block in the component so the
description stays linked consistently.
In `@app/web_ui/src/routes/`(app)/assistant/chat_status_steps.svelte:
- Around line 36-49: The compaction UI in `chat_status_steps.svelte` is
bypassing the active-turn guard because the `{`#if` compacting}` branch renders
before the `isLoading && isLastMessage` check. Update the conditional logic in
the `chat_status_steps` template so compaction only changes the displayed copy
when the current turn is active, rather than showing the spinner on a stale
`compacting` flag. Keep the `compacting` state as a message variant inside the
existing active-turn branch, and ensure the `showThinking`/other fallbacks
remain gated by the same `isLoading` and `isLastMessage` conditions.
---
Outside diff comments:
In `@app/web_ui/src/lib/chat/session_messages.ts`:
- Around line 47-58: The assistant snapshot handling in buildAssistantParts is
dropping reasoning-only content, which causes restored transcript entries to
disappear or lose the visible reasoning section. Update buildAssistantParts (and
any related content extraction in session_messages.ts) to preserve
msg.reasoning_content as a chat part alongside text and tool calls, so parts are
not empty for reasoning-only assistant messages and hydrated sessions match the
live transcript.
🪄 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: e9905b6c-6e03-4b3e-9c64-5a112e066cad
📒 Files selected for processing (22)
app/desktop/studio_server/chat/auto/runner.pyapp/desktop/studio_server/chat/auto/test_runner.pyapp/desktop/studio_server/chat/routes.pyapp/desktop/studio_server/chat/test_routes.pyapp/desktop/studio_server/chat/test_sse_parser.pyapp/web_ui/src/lib/api_schema.d.tsapp/web_ui/src/lib/chat/auto_run_store.test.tsapp/web_ui/src/lib/chat/auto_run_store.tsapp/web_ui/src/lib/chat/chat_history_apply.tsapp/web_ui/src/lib/chat/chat_session_store.test.tsapp/web_ui/src/lib/chat/chat_session_store.tsapp/web_ui/src/lib/chat/session_messages.test.tsapp/web_ui/src/lib/chat/session_messages.tsapp/web_ui/src/lib/chat/streaming_chat.test.tsapp/web_ui/src/lib/chat/streaming_chat.tsapp/web_ui/src/lib/ui/context_usage_gauge.svelteapp/web_ui/src/lib/ui/context_usage_gauge.test.tsapp/web_ui/src/routes/(app)/assistant/chat.svelteapp/web_ui/src/routes/(app)/assistant/chat_compacting_indicator.test.tsapp/web_ui/src/routes/(app)/assistant/chat_history.svelteapp/web_ui/src/routes/(app)/assistant/chat_status_steps.svelteapp/web_ui/src/routes/(app)/assistant/chat_status_steps.test.ts
✅ Files skipped from review due to trivial changes (1)
- app/web_ui/src/lib/api_schema.d.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- app/web_ui/src/lib/chat/auto_run_store.test.ts
- app/web_ui/src/lib/chat/chat_session_store.ts
- app/web_ui/src/routes/(app)/assistant/chat_history.svelte
- app/web_ui/src/routes/(app)/assistant/chat.svelte
- app/web_ui/src/lib/chat/auto_run_store.ts
- app/web_ui/src/lib/chat/streaming_chat.ts
Comment on lines
+232
to
+237
| enable_events = [ | ||
| e for e in client_events if e.toolName == ENABLE_AUTO_MODE_TOOL_NAME | ||
| ] | ||
| executable_events = [ | ||
| e for e in client_events if e.toolName != ENABLE_AUTO_MODE_TOOL_NAME | ||
| ] |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Mirror enable_auto_mode filtering in disable resolution.
This path excludes enable_auto_mode before execute_tool_batch, but _resolve_disable() still auto-executes all siblings. A mixed disable_auto_mode + enable_auto_mode batch can still produce the “Unknown tool name” result this change is avoiding.
Proposed fix shape
- siblings = [e for e in client_events if e is not disable_evt]
+ sibling_enable_events = [
+ e for e in client_events if e.toolName == ENABLE_AUTO_MODE_TOOL_NAME
+ ]
+ siblings = [
+ e
+ for e in client_events
+ if e is not disable_evt and e.toolName != ENABLE_AUTO_MODE_TOOL_NAME
+ ]
sibling_results = (
await execute_tool_batch(
[
@@
if siblings
else {}
)
tool_results = {
disable_evt.toolCallId: DISABLE_AUTO_MODE_RESULT,
**sibling_results,
+ **{
+ e.toolCallId: ENABLE_AUTO_MODE_RESULT
+ for e in sibling_enable_events
+ },
}🤖 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/chat/auto/runner.py` around lines 232 - 237, The
sibling filtering in runner.py is inconsistent: the `execute_tool_batch` path
excludes `enable_auto_mode`, but `_resolve_disable()` still auto-executes all
sibling events and can reintroduce the “Unknown tool name” case. Update
`_resolve_disable()` to apply the same `ENABLE_AUTO_MODE_TOOL_NAME` filtering
used in the batch split before resolving siblings, so mixed `disable_auto_mode`
and `enable_auto_mode` batches only process the executable events.
Comment on lines
+18
to
+21
| $: displayPercent = Math.min( | ||
| 100, | ||
| Math.round((usage?.context_percent ?? 0) * 100), | ||
| ) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clamp the lower bound too.
normalizeContextUsage() accepts any numeric context_percent, so a negative upstream value will render this meter with a negative aria-valuenow and width. Clamp to [0, 100] before deriving the styles.
Proposed fix
- $: displayPercent = Math.min(
- 100,
- Math.round((usage?.context_percent ?? 0) * 100),
- )
+ $: displayPercent = Math.max(
+ 0,
+ Math.min(100, Math.round((usage?.context_percent ?? 0) * 100)),
+ )Also applies to: 128-128
🤖 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/ui/context_usage_gauge.svelte` around lines 18 - 21, The
context usage gauge only caps the upper bound, so a negative `context_percent`
can still produce invalid `aria-valuenow` and width values. Update the
`displayPercent` calculation in `context_usage_gauge.svelte` to clamp the
normalized value from `normalizeContextUsage()` to the full [0, 100] range
before it is used for styling and accessibility attributes, and apply the same
fix anywhere the same calculation is duplicated.
Comment on lines
+101
to
+109
| <div | ||
| bind:this={triggerElement} | ||
| class="flex flex-col items-end gap-0.5 cursor-default" | ||
| role="meter" | ||
| aria-valuemin={0} | ||
| aria-valuemax={100} | ||
| aria-valuenow={displayPercent} | ||
| aria-label={`Approximately ${displayPercent}% of context used`} | ||
| data-testid="context-usage-gauge" |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Associate the tooltip with the focusable meter.
Keyboard focus shows the tooltip visually, but the trigger never references the role="tooltip" node, so screen readers only get the short aria-label and miss the explanatory text. Add a stable tooltip id and wire it with aria-describedby.
Also applies to: 134-143
🤖 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/ui/context_usage_gauge.svelte` around lines 101 - 109, The
focusable meter in context_usage_gauge.svelte is not associated with its
tooltip, so assistive tech only reads the short aria-label. Add a stable id to
the element rendered with role="tooltip" and reference it from the meter trigger
using aria-describedby on the bind:this={triggerElement} element, ensuring the
hover/focus tooltip text is exposed to screen readers. Update both the meter
markup and the tooltip block in the component so the description stays linked
consistently.
…om:Kiln-AI/Kiln into leonard/kil-686-eval-job
Drop GET /api/jobs/{id}/wait; the bulk GET /api/jobs/wait?ids=...
endpoint covers the single-job case with one id. Update stale doc
references and regenerate the OpenAPI schema.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHXGXnj1GKbFretPWeQWgP
Remove POST /api/jobs/{type} (and its wait=true/timeout inline-wait path):
production only runs evals, which use the typed POST /api/jobs/evals/run,
so the generic create endpoint had no real caller (only the temporary
test page + noop test worker). The NoopJobWorker stays as a registry-level
test fixture but is no longer registered in production.
Convert GET /api/jobs/wait to POST with a WaitForJobsRequest body, so the
job ids travel as JSON instead of repeated query params. With the generic
POST gone there's no route collision, so the ordering workaround is dropped.
Rewire test job creation to registry.create(), turn the temporary /jobs
test page into a real jobs panel (the jobs dialog links to it), and
regenerate the OpenAPI schema and agent-check annotations.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JHXGXnj1GKbFretPWeQWgP
Remove single-job wait endpoint in favor of bulk wait
…ience Chat resilience: transient-error retries (auto + interactive), side-note injection, and indicator polish
…nning Hitting Stop on an active auto run now opens a short confirmation dialog explaining that the agent won't start anything new, but jobs it already kicked off (like evals) keep running in the background — they're detached JobRegistry tasks the auto-run cancel never touches. The dialog only shows when there's a real server run; a brand-new armed (no-run) conversation just disarms silently since nothing could have started. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XfMWcQcB29iXj11cwU4oi
…ention Use the standard "Cancel" label for the cancel action (matching every other confirm dialog) instead of "Keep running". The dialog already uses the shared Dialog component and mirrors the sibling consent dialog's body structure. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XfMWcQcB29iXj11cwU4oi
Drop the wrapper div and leading-relaxed (carried over from the multi-paragraph consent dialog) — unnecessary for a single sentence. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019XfMWcQcB29iXj11cwU4oi
…button feat(chat): hard Stop button for auto mode (drop "waiting for you")
When a message is typed while a turn is in flight (interactive stream or a server-owned auto burst), hold it in a client-side queue instead of dropping it (non-auto) or firing it off immediately (auto). The composer surfaces the queued message above the input with three controls: - Send now (up arrow): interactive — stop the turn so it flushes immediately; auto — inject right away, keeping auto mode running (the runner picks it up at the next round boundary, which is the only "now" the backend supports without ending auto mode). - Discard (trash): drop the queued message. - Edit (pencil): pull it back into the composer to revise. A second send while one is queued appends (coalesces) rather than dispatching alongside it. The queued message auto-sends when the turn yields (interactive finish or auto burst idle) and then appears in the trace at the point it was actually sent. The composer textarea is no longer disabled during a turn. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
…to leonard/kil-queued-chat-message
…nner In auto mode, a message typed mid-burst no longer waits client-side for the whole auto run to go idle. It's injected immediately so the server-owned runner appends it to its next request (its next round boundary) — the "as soon as possible" path the inject endpoint is built for. The client-side queue/banner is now an interactive-only affordance (there's no server queue there, so it holds until the turn yields). Banner label reworded to "sends as soon as possible". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
Injecting on send made the message appear in the transcript immediately (the
server echoes the user-message at enqueue time), so it never looked "queued".
Now in auto mode a message typed during an active burst is held in the
client-side queue (one squashed entry) and injected at the runner's next round
boundary — detected via kiln-tool-execution-end, with auto-mode-idle as the
fallback for the final/text round. The echo then lands at that natural pause and
the runner appends the message to its next request, instead of jumping in
mid-round. When auto mode is idle ("waiting for you") there's no in-flight round,
so a send dispatches immediately.
Stopping the agent clears the pending queue (onAutoModeOff + the hard-stop path),
so nothing dangling is left behind. Send-now still injects immediately. The
interactive path is unchanged (hold until the turn finishes).
Banner label reworded earlier to "sends as soon as possible".
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
…d messages Two bugs surfaced by the queued-message feature when a message is injected into a live auto burst: 1. Duplicated round. The StreamEventProcessor accumulates a burst's parts and re-flushes the FULL set into the last assistant message on every event. When an injected user-message echo opens a fresh assistant turn mid-burst (beginAssistantTurn), the accumulators were never reset, so the next event re-flushed the prior round's text + tools into the new turn — duplicating it. Fix: add StreamEventProcessor.reset() and call it on the user-message echo so the new turn starts clean. 2. Message lost on hard refresh. resyncOnLoad hydrates from the run's current_trace_id and unconditionally replaced the transcript. But current_trace_id lags by a round while an injected message's response is still generating (the message only persists in the NEXT round's snapshot, and its optimistic echo is cleared from the replay buffer at the injection round's trace). So the stale snapshot clobbered the optimistic transcript, dropping the just-sent message until a full History reload. Fix: skip the destructive hydrate when current_trace_id is already present in the restored transcript (client is at/ahead of the registered leaf) and just re-attach. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
…anscripts Auto mode wraps a message injected mid-burst in a <system-reminder> "side note" before sending it upstream, so the framing is persisted in the trace. On reload (History restore / hard-refresh resync) the hydrated user message showed the raw tags, while the live echo renders the unwrapped content. Fix on the client, in hydrateSessionFromSnapshot, mirroring the existing stripAppUiContext handling for the <new_app_ui_context> header: add stripInternalFraming which strips both the app-UI context header and the auto-mode side-note <system-reminder>, so a hydrated transcript shows what the user actually typed. Keeping the framing in the persisted trace is intentional — it's a faithful record of the model's input — so no backend change is needed, and this also cleans up already-persisted conversations. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
… on approval Addresses PR review (Gemini + CodeRabbit): - Data loss: maybeFlush()/sendQueuedNow() cleared queuedMessage before actuallySend() confirmed success, so a declined consent, failed auto-mode injection, or pending armed enable silently dropped the user's typed text. Add dispatchQueued(), which clears optimistically then restores the message to the front of the queue if the send is rejected. - send-now vs tool approval: sendQueuedNow() only treated status !== "ready" as in-flight. Add a toolApprovalWaiter guard so it won't start a competing request while a pending-tool continuation's approval is open (status can be "ready" there); the message stays queued and flushes when that continuation yields. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
…scrollbar) The queued-message preview scrolled inside the text column, so its scrollbar sat to the LEFT of the edit/discard/send buttons and the text scrolled up under that row. Move the label + buttons into a full-width header row and make the scrollable text a full-width body below it, so the (thin, styled) scrollbar sits at the container's right edge instead of beside the buttons, and the buttons no longer overlap the scrolling text. Reuse the chat transcript's thin-scrollbar styling for the banner. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
…plication)
On a hard refresh mid-burst, the injected ("queued") message was duplicated, and
it compounded on each refresh. Cause: resyncOnLoad keeps the optimistic
transcript (which already shows the message), and the re-attach's buffer replay
re-emits the user-message echo for the still-in-flight round — appendEchoedUserMessage
then appended it again.
Give each injected message a stable id and render the echo idempotently:
- Server: InboundMessage gets a generated id (am_…); echo_user_message /
format_user_message include it in the user-message SSE event. (InboundMessage is
internal — built from SendMessageRequest — so the OpenAPI schema is unchanged.)
- Client: ChatMessage carries echoId; the user-message handler forwards it; and
appendEchoedUserMessage skips appending (and opening a new assistant turn) when
a message with that echoId is already present — whether it came from the
optimistic transcript or a buffer replay. This keeps the earlier
keep-optimistic resync fix (no message loss) while eliminating the duplicate.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
…p response loss) On a refresh mid-burst, part of the assistant response following a queued message was lost (recoverable only via History reload). Root cause: live rendering accumulates a whole burst into ONE assistant bubble, but on re-attach the buffer replay carries only the CURRENT in-flight round (the run buffer clears on every kiln_chat_trace) and no fresh assistant turn was opened for it — so the in-flight round's first flush (draft.parts = next, a full overwrite) clobbered the last existing bubble, destroying the rounds it held. Fix: - auto_run_store: on re-attach (resync / History restore), open a fresh assistant turn for the replayed in-flight round — lazily, on its first assistant content (or consumed by an injected-message echo, which opens its own turn), so an idle re-attach leaves no empty bubble. New optional attach(openInflightTurn) arg; resync and History restore pass it, the initial burst attach does not. - resyncOnLoad: revert the keep-optimistic branch back to always adopting the snapshot (per-round bubbles), which composes cleanly with the fresh in-flight turn — the snapshot owns completed rounds, the buffer replay owns the in-flight one. Keeping the multi-round optimistic bubble would instead duplicate the in-flight round. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T8SUTZJG6Zjp5p5kdxLz6R
feat(chat): queue messages sent mid-turn with send-now / edit / cancel
…om:Kiln-AI/Kiln into leonard/kil-686-eval-job # Conflicts: # app/desktop/studio_server/jobs/models.py # app/web_ui/src/lib/api_schema.d.ts # app/web_ui/src/lib/components/jobs_table.svelte
Populate Field(description=...) on EvalJobParams, EvalJobResult, and EvalJobProperties in the eval worker. EvalJobParams is the typed request body for POST /api/jobs/evals/run, so its descriptions flow into the generated OpenAPI schema; the result/properties descriptions document the models carried as generic dicts on JobRecord. Regenerated api_schema.d.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Conw7oxpDtZTfhtEtVS9gT
leonardmq
changed the title
…1536) * Add failing-train-examples eval API for reflective optimization New endpoint POST .../evals/{eval_id}/eval_config/{eval_config_id}/failing_train_examples that samples an Eval's train set, runs its judge (EvalConfig), and returns the datapoints that fail — with the judge's plaintext feedback — to feed GEPA-style reflection loops. - libs/core/.../eval/failing_examples.py: find_failing_train_examples() shuffles the train set (eval.train_set_filter_id), judges items in concurrent batches via the eval_config_eval path, and stops once `count` failures are found or `max_samples` items are judged ("oversample, return the requested amount"). An example fails only when all output scores fall below the bar (normalize_rating < threshold, default 0.75). Results are persisted as EvalRuns and reused on later calls. - eval_api.py: thin endpoint + request/response models, allowed for the Kiln assistant (ALLOW_AGENT) with a detailed OpenAPI description. Committed the agent-policy annotation so the assistant's policy lookup permits the call. - Regenerated app/web_ui api_schema.d.ts. - Tests: 13 core + 6 API. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: reuse loaded eval, resilient persistence, more tests - eval_api.py: resolve the eval config from the already-loaded eval instead of eval_config_from_id (which re-reads the task/eval from disk). Keeps the same 404. - failing_examples.py: wrap EvalRun persistence in its own try/except so a save failure logs and is skipped instead of crashing the whole concurrent batch; the computed scores are still returned. - tests: cover missing scores in example_fails, and judge errors being skipped (still counted as examined) during orchestration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Rework into a persistent Judge Job (config + run + poll) Replace the stateless failing_train_examples endpoint with a durable, runnable JudgeJob model per review feedback. A JudgeJob samples dataset items by tag, judges their existing outputs with an eval config (the judge), and records each item's pass/fail + the judge's feedback as child JudgeJobRuns — surfacing failing examples for reflective optimization. - datamodel/judge_job.py: JudgeJob (child of Task, parent of JudgeJobRun) with target_tags, eval_config_id, run_config_id (metadata), count/max_samples/threshold, latest_status, and an outcome summary. Registered on Task + datamodel __init__. - adapters/eval/judge_job_runner.py: JudgeJobRunner mirrors EvalRunner — judges in eval_config_eval mode, yields Progress for SSE, persists child runs, reuses cached results, and updates status/outcome (running -> succeeded/failed). Keeps the example_fails/score_passes/feedback helpers from the prior engine. - studio_server/judge_job_api.py: create / run (SSE) / create-and-run (SSE) / get / runs / list. The model id is the job id; GET is the poll. Registered in desktop_server; "Judge Jobs" tag added; agent-policy annotations regenerated (ALLOW_AGENT). Removed the standalone endpoint and its annotation; regenerated api_schema.d.ts. - Tests: datamodel, runner, and API (incl. SSE). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: cancel on disconnect, validation, defensive hardening - judge_job_runner: catch GeneratorExit/CancelledError and mark the job `cancelled` (an SSE disconnect previously left it stuck in `running`); add a test. - judge_job_runner: harden score_passes (catch TypeError), feedback extraction (skip non-str values), and tag matching (None-safe). - judge_job_api: validate run_config_id (if provided) and reject a second run with 409 when the job is already running; add tests. - judge_job datamodel: constrain count/max_samples (ge=1) and threshold (0-1). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Make judge jobs synchronous (per Leonard's feedback) The chat tool drops SSE event payloads and a judge minibatch is short, so SSE buys nothing here. Switch run / create-and-run to synchronous JSON and drop the job-status/poll/cancel layer. - judge_job datamodel: remove latest_status, outcome, JudgeJobStatus, JudgeJobOutcome. A JudgeJob is now just a config (+ JudgeJobRun children). - judge_job_runner: run() returns a JudgeJobRunResult (failing_runs + counts) instead of streaming Progress; no status writes, lock, or GeneratorExit handling. - judge_job_api: run / create-and-run block and return JudgeJobRunResponse (judge_job + failing_runs + counts). Counts are FYI for the caller, not persisted. Removed the SSE helper and the 409 already-running guard. - Tests + api_schema.d.ts updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address Leonard's review: rename task_run_id, collect per-item errors - Fix CI lint failure (ruff format test_judge_job.py). - Rename JudgeJobRun.dataset_id -> task_run_id (more descriptive of the TaskRun it points at). Updates model, runner, API, tests, and regenerated TS schema. - Collect per-item judge/save errors during a run and return them in the sync response (new JudgeJobItemError + JudgeJobRunResult.errors / run-response `errors`). Errors no longer silently swallowed: one bad item doesn't abort the run, the item is left un-persisted, and re-running retries only un-persisted items. A non-empty `errors` list signals partial success to the caller. Per-run aggregate counts stay derived (returned, not persisted) — the durable record remains the per-item JudgeJobRun children. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Judge jobs: full-coverage gate + return all judged runs Replace `count` (always early-stops) with `stop_after_failures: int | None`: - None (default) = judge the whole matching set up to max_samples (full coverage), so a val gate can pair results by task_run_id. - set = stop once that many failures are found (the cheap train-signal minibatch). Add `judged_runs` (every item judged this run, pass and fail) to the run result and response, keyed by task_run_id — the piece that makes a paired baseline-vs-candidate gate computable instead of an aggregate-count approximation. hit_cap now means coverage was capped (max_samples reached before stop_after_failures, or the matching set exceeded max_samples). Regenerated api_schema.d.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Judge jobs: retry transient per-item errors Generating/judging invokes a model, so transient rate-limit/connection blips are expected; without a retry they were collected as per-item errors and silently shrank coverage (skewing a gate). Route the judge call through _judge_with_retry, reusing the eval runner's transient-error classification (max_retries=2). Non- transient errors are still collected once, not retried. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Judge jobs: generate-and-judge mode (scoped candidate gate) Add generate_outputs: when true, run run_config_id on each tagged item to produce a fresh output and judge that — gating a candidate config scoped to the tagged items, in one sync call (no run_comparison, no full-eval scores). run_config_id is required in this mode. - Runner resolves the run config's RunConfigProperties + preloads its skills (mirrors EvalRunner.run_job) and instantiates the evaluator with them. - _judge_with_retry branches to run_task_and_eval; the fresh TaskRun is discarded (allow_saving=False) so the dataset is never polluted. - Skip the result cache in generate mode (generation is non-deterministic). - Record run_config_id on each JudgeJobRun for provenance; lower default concurrency when generating. Regenerated api_schema.d.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Judge jobs: address review (approval, robustness, eval-type, naming) Per Leonard's review: - Run / Create-and-run endpoints now require agent approval (agent_policy_require_approval) — they make model calls, so the bot shouldn't kick them off without consent (auto-mode still bypasses). Create + GETs stay ALLOW_AGENT. Regenerated the two annotation files. - run()'s asyncio.gather now uses return_exceptions=True: an unexpected throw in one item is converted to a per-item error instead of discarding the whole chunk's results. - Reject reference-answer evals when generate_outputs=false (no reference to compare a pre-existing output against; the judge would error per item). Validated in the API (422) and the runner constructor (last line of defense). - Rename eval_config_for_id -> eval_config_from_id to match the *_from_id convention. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Rename JudgeJob -> JudgeFeedbackBatch Avoid collision with the upcoming Job management system (and GEPA's "job"), per Leonard's review. Mechanical rename across the datamodel (JudgeFeedbackBatch / JudgeFeedbackBatchRun), runner, API (paths: /judge_jobs -> /judge_feedback_batches), the Task accessor (judge_feedback_batches()), OpenAPI tag, files, tests. Regenerated api_schema.d.ts and the agent-policy annotations (removed the stale judge_jobs files — the endpoints never shipped). Zero external callers, so no compat shim. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Judge feedback batch: return continuous per-dimension scores (P1) The pass/fail bit (example_fails at the 0.75 threshold) discards the continuous signal, so a 2★→3★ improvement reads as zero gradient and a small val slice quantizes to a few loss levels. Add aggregate_normalized_scores() and return mean_normalized_scores (per-dimension mean over judged_runs, 0-1 higher=better) + mean_normalized_score (overall) in the run response — a usable gate/loss metric the caller no longer has to hand-compute from judged_runs[].scores. Part of the loss-function API review; P2/P3/P4/P7 are a separate follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * judge_feedback_batch: surface generation usage (tokens/cost/latency) Stop discarding the candidate run's Usage in generate_outputs mode. JudgeFeedbackBatchRun now carries a per-item `usage`; the runner aggregates it (aggregate_usage) into total_usage / mean_cost / mean_latency_ms on JudgeFeedbackBatchRunResult, and the run API exposes both per-run and aggregate usage. Lets the auto-optimize loop read deterministic cost/latency signals from the same call that gates quality. None on the judge-only path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * judge_feedback_batch: exponential backoff on transient/rate-limit retries Fixed 1s gaps don't let a 429 clear and just re-flood a throttled provider — back off progressively (delay, 2x, 4x) in _judge_with_retry, and raise the default retry_delay 1.0->2.0 so the server-side backoff is gentler. Surfaced by an auto-optimize smoke whose single-sample judge_feedback_batches calls hit provider rate limits (Cerebras/Gemini-Flash candidates). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(jobs): add judge_feedback_batch job type (parallel kick-offs) Make judge feedback batch runs job-backed, alongside the existing synchronous endpoints, so an agent can fire many gates and POST /api/jobs/wait on all of them at once instead of blocking on each synchronous call serially. - JudgeFeedbackBatchJobWorker wraps JudgeFeedbackBatchRunner unchanged (mirrors EvalJobWorker). Single-shot: progress reported once, supports_pause=False. - POST /api/jobs/judge_feedback_batch/run (two-segment, approval-gated) — the job-backed counterpart to POST /judge_feedback_batches/run. - Result carries the aggregate scores/usage/latency + the batch id; per-item runs (with the judge's feedback) are persisted, fetched via .../runs. - Worker tests + regenerated api_schema.d.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(jobs): agent-check annotation for the judge_feedback_batch job route The chat backend gates call_kiln_api via AgentPolicyLookup, which reads the static agent-check annotation JSON (dumped from the OpenAPI), NOT the live spec. Without an annotation the new POST /api/jobs/judge_feedback_batch/run is rejected as "not allowed", so the assistant falls back to the synchronous endpoint. Regenerated via `make annotations`; marks it allow + requires_approval. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Judge feedback batch: job-backed run of a pre-existing batch (eval parity) + deep-review fixes (#1542) * feat(jobs): judge_feedback_batch worker — stream progress, publish props, describe Bring the judge_feedback_batch job worker to parity with the eval job worker (the auto-mode e2e integration flagged three gaps): - Progress: the worker reported progress only once at the end, and used train_set_size as the denominator — so when the matching set exceeded max_samples the bar stalled below 100% even on full success. The runner now takes an optional progress_callback and streams (num_judged, error_count, planned_total) per judged chunk; the worker reports live progress and its final snapshot against the planned (capped) count min(train_set_size, max_samples), so success reaches total on full coverage. - Metadata: publish JudgeFeedbackBatchJobProperties via describe() (mirrors EvalJobProperties) — judge/eval names, algorithm, model, mode, run config, tags, max_samples — and render them in the jobs table (previously just a raw "Judge_feedback_batch" label). - Errors: unchanged wiring already surfaces per-item errors to the View Errors UI; improved message quality by unwrapping KilnRunError.original (mirrors EvalJobWorker._error_detail). - API models: add field descriptions to JudgeFeedbackBatchJobResult and the project_id/task_id params. Regenerated api_schema.d.ts. Tests: runner progress_callback streaming, worker describe() + capped-total progress, frontend judge-property rendering. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZXQFWTXbstDj7vBihcx5N * fix(judge-feedback-batch): address deep code review (6 moderate + polish) Deep multi-phase review of the judge_feedback_batch feature (0 critical). Fixes: Moderate: - Progress double-counted errored items: the runner's num_judged already includes errored items, so reporting success=num_judged alongside a separate error count breached the processed=success+error contract and overshot 100%. Report success=num_judged-error_count in both the streamed callback and the final snapshot (worker), matching the eval reference's disjoint counters. - Gate-mode task_run_id pairing was defeated by the random shuffle when the tag set exceeded max_samples (two runs sampled disjoint subsets). Gate mode now selects deterministically (sorted by id) before capping; train-signal mode keeps the random minibatch. - Added a JudgeFeedbackBatch model_validator coupling generate_outputs=True to a required run_config_id and rejecting empty target_tags (defense in depth, mirrors EvalRun) — the API request model already validated both. - Documented that num_judged counts attempts (errors/cache included); use len(judged_runs) for a scored-item count. - Added runner tests for the empty candidate set and gate-mode hit_cap=True (set > max_samples), incl. proof that two gate runs cover the same ids. Polish: - Runner: save-error uses _error_detail; concurrency=max(1, concurrency) guard; mean-of-means comment; "Judge feedback batch" wording. - API: judge_feedback_batch_from_id 404 message fixed + accepts a preloaded task (removes the run endpoint's double task load); run docstring covers generate mode. - UI: surface eval_name + stop_after_failures in the jobs table. - Tests: worker run stub gets the full signature; judge-only describe() test. Full checks.sh green. Regenerated api_schema.d.ts. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZXQFWTXbstDj7vBihcx5N * fix(judge-feedback-batch): report per-item errors live so View Errors works mid-run The streamed progress callback bumps progress.error per chunk, which makes the "View Errors" button appear while the job is still running. But the error MESSAGES were only written to the job error log in a post-run loop over result.errors — after runner.run() returned. So mid-run the button showed but the log was empty ("No error messages recorded"); the messages only appeared once the job completed. This diverged from the eval worker, which logs each error live via an observer. Add an error_callback to JudgeFeedbackBatchRunner.run(), fired the moment each per-item error is collected (both the judge/save error and the unexpected-throw paths), and wire the worker to write it to the error log immediately. Errors still appear in the returned result.errors for the synchronous endpoint (which passes no callback). Removed the worker's post-run loop to avoid double-logging. Tests: runner test asserting errors are delivered live (interleaved with progress, not batched to the end); worker stub updated to emit errors via the callback. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZXQFWTXbstDj7vBihcx5N * fix(jobs-table): guard target_tags render against missing properties Defensive: use optional chaining on jp.target_tags so a job record with incomplete properties can't throw a TypeError and crash the whole jobs table render. (Per gemini-code-assist PR review.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZXQFWTXbstDj7vBihcx5N * fix(judge-feedback-batch): wrap batch-config save in git-sync context The batch config was written with a raw save_to_file() outside any atomic_write — in both the job worker and the synchronous create-and-run endpoint. Under auto git-sync that new (untracked) file is dirty working-tree state, so the runner's first per-item child write enters atomic_write, whose ensure_clean() treats it as a crashed session and stashes it away (include_untracked=True). Net effect: the batch config is never committed/pushed and is removed from the working tree, orphaning the committed child runs. Wrap the batch-config save in the same save_context used for the child writes (coalescing None -> no-op default_save_context) so it gets its own atomic_write / commit before the runner starts. The batch save and each child save are separate, non-nested atomic_write blocks, so re-entrancy is not a concern. Mirrors how EvalRunner already wraps every per-item write (the eval worker creates no parent entity, so it was already correct). - worker: judge_feedback_batch.py run() wraps the save. - sync endpoint: create_and_run_judge_feedback_batch (@no_write_lock) wraps it via build_save_context(request). - test: asserts the batch save goes through the git context and closes before the runner runs (enter -> exit -> run). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZXQFWTXbstDj7vBihcx5N * refactor(judge-feedback-batch): job runs a pre-existing batch (eval parity) Reshape the judge_feedback_batch job from "create-and-run" to "run an existing batch", mirroring EvalJobWorker. The batch config is created first via the synchronous POST .../judge_feedback_batches (which persists it under the git-sync write lock), then the job just runs it by id. Why: - The batch id is now caller-supplied and lives in job.params, so it's retrievable via GET /api/jobs/{id} even if the run fails — closing the gap where a create-and-run job only surfaced the minted id in its success result. - The worker no longer writes the config at all, so the earlier "wrap the batch save in the git-sync context" workaround is gone — the create endpoint owns that write. One less special case. - Structurally identical to the eval job (params carry the entity id; results live on disk), which sets up deriving a disk summary + a real compute_state later. Changes: - JudgeFeedbackBatchJobParams: now {project_id, task_id, judge_feedback_batch_id} (was a full CreateJudgeFeedbackBatchRequest). run()/describe() load the batch by id and read its config off disk. - Job result echoes judge_feedback_batch_id from params (no longer "created"). - Route docstring updated; regenerated api_schema.d.ts. - Tests: run loads an existing batch, missing-batch 404s, describe/props derived from the persisted batch. Dropped the now-obsolete config-save-wrap test. The synchronous create-and-run / run endpoints are left in place (now redundant with create + job) for callers not yet migrated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZXQFWTXbstDj7vBihcx5N * chore(judge-feedback-batch): deny agent access to the sync POST endpoints The synchronous create / run / create-and-run endpoints are being superseded by the create + job flow, so mark them DENY_AGENT (the chat assistant should go through POST /api/jobs/judge_feedback_batch/run for dashboard visibility). GETs stay ALLOW_AGENT. Drops the now-unused agent_policy_require_approval import. NOTE: not enforced until the agent-check annotation JSONs are regenerated (the policy lookup reads those, not the live spec). See PR notes re: keeping the create endpoint agent-callable for the new create-then-run-job flow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZXQFWTXbstDj7vBihcx5N * chore(judge-feedback-batch): keep create agent-callable; regenerate annotations Follow-up to the DENY_AGENT change: - Flip the sync create endpoint back to ALLOW_AGENT — the agent creates the batch here (cheap, no model calls) and then runs it via POST /api/jobs/judge_feedback_batch/run. Only the sync run / create-and-run stay DENY_AGENT (superseded by the job). - Regenerate the agent-check annotation JSONs so the policy is actually enforced (the chat backend reads the dumped JSONs, not the live spec) — fixes the check_api_bindings CI job. run / create-and-run now dump as "deny"; create and the GETs stay "allow". Final judge_feedback_batch agent policy: create -> allow run (existing, sync) -> deny create-and-run (sync) -> deny run as job (/api/jobs/...) -> allow + approval list / get / runs (GET) -> allow Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015ZXQFWTXbstDj7vBihcx5N --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * feat(judge-feedback-batch): expose concurrency in job params Add an optional `concurrency` field to JudgeFeedbackBatchJobParams and forward it to JudgeFeedbackBatchRunner.run. Null keeps the runner's mode-aware default (5 when generating outputs, 25 when judging existing ones); values below 1 are clamped to 1 by the runner. Regenerated api_schema.d.ts and added a param-forwarding test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019w3CFeeewrMyMRdTv3SgQm * feat(judge-feedback-batch): validate concurrency >= 1 at schema level Add ge=1 to the concurrency param so invalid input returns a 422 up front (consistent with max_samples / stop_after_failures) instead of being silently clamped by the runner. Update the description, regenerate api_schema.d.ts, and add a validation test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019w3CFeeewrMyMRdTv3SgQm * feat(eval-job): expose concurrency in job params Add an optional `concurrency` field to EvalJobParams and forward it to EvalRunner.run, mirroring the judge feedback batch job param. Null keeps the runner's default (25); ge=1 rejects invalid values with a 422 at the API boundary (matching max_samples-style validation) rather than a runner-side ValueError. EvalRunner.run now accepts int | None and resolves the default internally, keeping 25 a single source of truth. Regenerated api_schema.d.ts; added forwarding + validation tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019w3CFeeewrMyMRdTv3SgQm * fix(eval-job): CI + review — params round-trip and single-source default - test_api: _EVAL_PARAMS now includes the defaulted concurrency=None, so the stored-params round-trip assertion in test_run_eval_job_creates_typed_eval_job matches again (the new optional field is serialized into job.params). - Address gemini review: extract DEFAULT_EVAL_CONCURRENCY (=25) in eval_runner as the single source of truth for the default, use it in EvalRunner.run and interpolate it into the EvalJobParams.concurrency field description so the doc can't drift from the runner default. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019w3CFeeewrMyMRdTv3SgQm --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Leonard Q. Marcq <marcqleonard@gmail.com> Co-authored-by: Leonard Q. Marcq <leonard@getkiln.ai>
Base automatically changed from
leonard/kil-686-feat-background-job-system
to
main
July 2, 2026 15:55
…to leonard/kil-686-eval-job
leonardmq
changed the title
…etry schedule The retry machinery in EvalRunner and JudgeFeedbackBatchRunner never fired in production: the model adapter wraps provider exceptions in KilnRunError, so _is_retryable_error's isinstance checks against litellm error types never matched. Rate-limited items failed on the first attempt and went straight to the job error log. The classifier now unwraps KilnRunError and classifies the underlying error. Retry backoff is now exponential with +/-50% jitter (shared jittered_backoff_delay) instead of a fixed delay, so concurrent workers throttled at the same moment don't retry in lockstep and re-flood the provider. EvalRunner.run() exposes max_retries/retry_delay (keeping the historical defaults of 2 retries), and both background job workers override with a more patient schedule (4 retries, 5s base -> ~5/10/20/40s jittered waits). The RetryableError raised from run_job now carries the underlying provider message instead of KilnRunError's genericized text, keeping View Errors detailed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNMsZVT2b9PuANGAHkPyb1
…d error detail Addresses review: guards a (contract-violating) None original, and keeps the detail extraction from diverging from the classifier on nested wraps. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JNMsZVT2b9PuANGAHkPyb1
…etries Fix dead transient-error retries; patient backoff for background jobs
# Conflicts: # libs/core/kiln_ai/adapters/eval/test_eval_runner.py
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
3 participants
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.
This is the app branch behind KIL-686 (background jobs) that has grown to carry the whole assistant-driven eval/optimize workflow. It began as "run an eval as a background job" and consolidated several stacked PRs — auto mode (#1518), the context gauge / compaction indicator (#1519), the jobs-API cleanups (#1527), auto-mode resilience (#1529), worker properties (#1530), chat retry + Stop (#1532/#1533), queued messages (#1534), and the
judge_feedback_batchsubsystem (#1536) — into a single branch so onedesktop_serverserves everything the merged kiln-chat skill needs.No existing eval UI is changed; the eval/judge work is purely additive (new background-job and batch paths for agents).
Main features
EvalJobWorker+ a typed, non-streamingPOST /api/jobs/evals/runkickoff for agents, plus a bulkPOST /api/jobs/waitto await many jobs at once. Agents use these instead of the SSE eval endpoints.(eval_config, run_config, slice)→ per-item judge scores / usage / latency subsystem: datamodel, runner, a synchronous REST API, and a job-backed worker so batches can run inline or through the job system.chat/auto/app-server subsystem and matching web UI.Agent permissions
Agent-allowed job endpoints:
POST /api/jobs/evals/run(allow + approval — evals cost AI credits) andPOST /api/jobs/wait(allow). The UI-only/api/chat/auto/*set and the SSErun_comparison/run_calibrationeval endpoints are agent-forbidden (deny). Regenerated agent-check annotations and the OpenAPI TS schema throughout.Testing
New worker, endpoint, registry, judge-batch, auto-mode, retry, queued-message, context-gauge, and compaction-indicator tests across Python and web.
uv run ./checks.sh --agent-mode→ green (Python lint/format/type/tests, web lint/format/check/test/build, OpenAPI schema in sync).Changelog
Linear with the commit history — each entry is one holistic increment (roughly one merged sub-PR / ticket). Append new entries at the bottom.
1. Run evals as background jobs
EvalJobWorker(jobs/workers/eval.py) wraps the existingEvalRunnerso an eval runs in the background. Idempotent,supports_pause=True(EvalRunner excludes already-run(eval_config, run_config, dataset)triples), progress against full eval-set size.POST /api/jobs/evals/run— typed, non-streaming kickoff (body is the 5 eval ids; returns{job_id, status}). Allow + requires-approval. Two-segment path so it can't be shadowed by the generic job route.run_comparison,run_calibration) flipped to agent-forbidden.JobDerivedState.erroris nowint | None; a worker that can't derive its error count from entities returnsNone, and_apply_derivedkeeps the live reported count instead of clobbering it to0.2. Assistant auto mode (rescoped, #1518)
chat/auto/subsystem: auto-run registry, runner, event bus, SSE, and API (/api/chat/auto/*: enable, decline, resolve, sessions, per-run message/stop/events).enable_auto_mode/disable_auto_modebuilt-in tools + tool-registry wiring.auto_run_store, consent dialog, chat-history grouping, reworkedchat.svelte(in-transcript live working/idle, inject-on-send, reconnect handling), preserving the base's UI refactors (scrollbar-to-side, DaisyUI send/stop, centering wrapper).jobs/events.py(KeepalivePing/iter_with_keepalive, generic over aTypeVar).Literal["user"]role, optimistic-flag reset on inject failure, consent-event trace fallback, keyboard-focus reveal,consentPendingheld throughrequestEnable, and a guard soresyncOnLoadcan't overwrite a newly-selected session on a mid-flight conversation switch.3. Multi-job wait + eval per-item error logging
JobRegistry.wait_many()— blocks until all given jobs are terminal and returns their records. Lets a caller await several eval jobs in one call.EvalRunner.run()now accepts observers;EvalJobWorkerforwards each failed dataset item's exception toctx.report_error(withdataset_id+run_config_id). Previously only the error count was reported, soGET /api/jobs/{id}/errorsshowed "no errors recorded" even when every item failed.4. Context-usage gauge + compaction indicator — KIL-727 (#1519)
ChatSessionSnapshotgains acontext_usagefield (context_tokens,context_limit,context_percent,compacted) so a session reopened from history renders the gauge. Relies on Pydantic's defaultextra="ignore"(made explicit) to drop unmodeled upstream keys at the client boundary (extra="forbid"deliberately avoided). SSE stays a raw passthrough; passthrough tests added.context_usage_gauge.svelte) — a small grey bar in the input footer (opposite Auto mode), percentage above and a tooltip showing approximateused / totaltokens; hidden before the first turn. Token counting is intentionally approximate (overestimate-biased; copy uses "≈"). Tooltip fixed to measure after visible and made keyboard-accessible.finished(a fast started→finished pair would collapse the window); cleared by the first real assistant content, on error, and on reset.streaming_chat.ts/chat_session_store.ts/auto_run_store.tsso the gauge updates during auto-mode bursts too.5. Auto/manual congruence + gauge polish
chat/auto/runner.py).6. Jobs-API consolidation — bulk wait only (#1527)
GET /{id}/wait(bulk wait covers it with one id) and the genericPOST /api/jobs/{type}create endpoint (production only runs evals, via the typed route — the generic path had no real caller).GET /api/jobs/waitconverted toPOST /api/jobs/waitwith aWaitForJobsRequestbody (ids as JSON, not repeated query params); with the generic POST gone, the route-ordering workaround is dropped./jobstest page becomes a real jobs panel;NoopJobWorkerstays as a registry-level test fixture but is no longer registered in production.7. Auto-mode resilience — KIL-692 (#1529)
<system-reminder>telling the model to weave its reply into ongoing work and keep going, so a quick aside no longer settles the burst IDLE.auto-mode-retrySSE event drives a "retrying N/M…" transcript affordance.USER_STOPPED(publishes auto-mode-off) instead of leaving the flag on.8. Worker properties + eval summary in the jobs table (#1530)
compute_statepure-read pattern; stored dict-on-the-wire likeprogress_detail, computed behind a guard so a failingdescribe()never breaks job creation).describe()→ type-check →model_dump()path wrapped so a bad payload can never breakcreate(); undeclared-model case made explicit; test coverage for the contract guard and the previously-dead prompt-resolution branches.9. Surface the original eval error (#1531)
KilnRunError.originalsoGET /api/jobs/{id}/errorsshows the real failure instead of the generic "An unexpected error occurred." wrapper text.__str__on the original exception (falls back to the class name), and covered the commonRetryableError(str(original))path.10. Chat retry in both modes + hard Stop (#1532, #1533)
iter_round_with_retries(stream_session.py) owns the retry loop and is used by both the auto runner and interactiveChatStreamSession.stream()— previously only auto mode retried. Backoff is an explicit ramp capped at 60s (1, 2, 5, 10, 20, 30, 60, 60, 60, 60, ±15% jitter, ~5 min total);MAX_CHAT_RETRIESderives from its length.auto-mode-retry → kiln-chat-retry; the frontend renders "Temporary issue — retrying N/M…" from either source. Stop is honored during the backoff (no re-POST).auto_mode_stop_dialog.svelte(notes that already-kicked-off background jobs keep running).11. Mid-turn message queueing (#1534)
<system-reminder>side-note framing from hydrated transcripts.12. Eval job model docs
Fielddescriptions to the eval job models.13. Judge feedback batch subsystem (#1536)
Merges the
judge_feedback_batchcapability so one branch serves both jobs and judge for the auto-optimize skill.JudgeFeedbackBatch(libs/core/.../datamodel/judge_feedback_batch.py) andJudgeFeedbackBatchRunner(.../adapters/eval/judge_feedback_batch_runner.py): a(eval_config, run_config, slice)→ continuous per-dimension scores, per-item judge reasoning, generation usage (tokens/cost/latency), with exponential backoff on transient/rate-limit retries.judge_feedback_batch_api.py) — create / run / create-and-run / list / get / get-runs under/api/projects/{project_id}/tasks/{task_id}/judge_feedback_batches. Cancels on client disconnect; full-coverage gate; generate-and-judge mode (scoped candidate gate).JudgeFeedbackBatchJobWorkerruns a pre-existing batch through the job system (supports_pause=False, per-chunk progress) alongside the synchronous path.concurrencyparam (feat(judge-feedback-batch): expose concurrency in job params #1544, feat(eval-job): expose concurrency in job params #1545).14. Structured add-example dialog (#1540, via main)
$lib/components/add_example_dialog.sveltewith configurableinclude_input/include_outputand schema-aware, field-by-field entry (RunInputFormElement) for structured tasks; plaintext tasks keep the textarea. The route-local copy is deleted and callers consume the shared component + exportedGuideSampletype.🤖 Generated with Claude Code