assistant: auto mode reflective optimization prototype (rescoped onto eval-job)#1518
Merged
leonardmq merged 4 commits intoJun 25, 2026
Conversation
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
Contributor
WalkthroughAdds server-owned auto-mode across the backend, web UI, core tools, jobs, and specs. It introduces new auto-mode endpoints, run management, client consent/resync flows, job waiting, tool wiring, and updated session/history rendering. ChangesAssistant Auto-Mode and Job Wait
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
📊 Coverage ReportOverall Coverage: 92% Diff: origin/leonard/kil-686-eval-job...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 245-253 245 if run_id is None:
246 return None
247 run = self._runs.get(run_id)
248 if run is None:
! 249 return None
250 # A run is reachable via the trace index only once it has a leaf (set at
251 # start for a trace-seeded run, or by _on_trace on the first
252 # kiln_chat_trace), so current_trace_id is non-None here. A no-trace seed
253 # (Revision R2) isn't indexed until its first trace arrives, so it can'tLines 253-261 253 # (Revision R2) isn't indexed until its first trace arrives, so it can't
254 # be resolved before then — guard defensively for the type checker.
255 current_trace_id = run.record.current_trace_id
256 if current_trace_id is None:
! 257 return None
258 return (run_id, current_trace_id, run.record.status)
259
260 # -- start ---------------------------------------------------------------Lines 330-338 330
331 def _fresh_run_id(self) -> str:
332 run_id = _new_run_id()
333 while run_id in self._runs:
! 334 run_id = _new_run_id()
335 return run_id
336
337 # -- inbound message injection (Revision R1) -----------------------------Lines 381-389 381 directly. Publishes auto-mode-off(user_disabled). Returns False if the
382 run is unknown."""
383 run = self._runs.get(run_id)
384 if run is None:
! 385 return False
386
387 # Mark USER_DISABLED first so a cancelled burst's CancelledError handler
388 # preserves it (rather than forcing USER_STOPPED) and publishes the
389 # correct off-reason.Lines 387-395 387 # Mark USER_DISABLED first so a cancelled burst's CancelledError handler
388 # preserves it (rather than forcing USER_STOPPED) and publishes the
389 # correct off-reason.
390 if run.record.status.is_terminal:
! 391 return True
392 run.record.status = AutoRunStatus.USER_DISABLED
393
394 task = self._tasks.get(run_id)
395 if task is not None:Lines 397-406 397 try:
398 await task
399 except asyncio.CancelledError:
400 pass
! 401 except Exception:
! 402 logger.debug(
403 "Auto run %s raised during disable await", run_id, exc_info=True
404 )
405 return TrueLines 442-455 442 run.inbound.clear()
443 self._publish_off(run)
444 self._touch(run)
445 raise
! 446 except Exception:
447 # An unrecoverable burst error ends the burst but leaves the flag
448 # on so the user can retry or stop (functional spec §4.4).
! 449 logger.exception("Auto run %s burst failed", run_id)
! 450 run.runner.idle_reason = "error"
! 451 run.record.status = AutoRunStatus.IDLE
452 else:
453 # If the flag was already cleared off-band (e.g. disable() raced
454 # in just as the burst returned), don't resurrect it — leave the
455 # terminal status as-is. Otherwise adopt the runner's status.Lines 488-496 488
489 def _on_trace(self, run_id: str, new_trace_id: str) -> None:
490 run = self._runs.get(run_id)
491 if run is None:
! 492 return
493 if new_trace_id not in run.record.seen_trace_ids:
494 run.record.seen_trace_ids.append(new_trace_id)
495 run.record.current_trace_id = new_trace_id
496 self._trace_index[new_trace_id] = run_idLines 533-541 533
534 def _schedule_gc(self, run_id: str) -> None:
535 existing = self._gc_tasks.get(run_id)
536 if existing is not None and not existing.done():
! 537 return
538 self._gc_tasks[run_id] = asyncio.create_task(self._gc_after_ttl(run_id))
539
540 def _cancel_gc(self, run_id: str) -> None:
541 """Defensive: a fresh burst should never start on a GC-scheduled runLines 542-550 542 (only off runs are GC'd, and a new burst only starts from IDLE), but
543 cancel any pending GC if one exists."""
544 gc_task = self._gc_tasks.pop(run_id, None)
545 if gc_task is not None:
! 546 gc_task.cancel()
547
548 async def _gc_after_ttl(self, run_id: str) -> None:
549 try:
550 await asyncio.sleep(TERMINAL_TTL_SECONDS)Lines 556-564 556
557 def _evict(self, run_id: str) -> None:
558 run = self._runs.pop(run_id, None)
559 if run is None:
! 560 return
561 for tid in list(self._trace_index):
562 if self._trace_index[tid] == run_id:
563 del self._trace_index[tid]
564 logger.debug("Evicted terminal auto run %s after TTL", run_id)app/desktop/studio_server/chat/auto/runner.pyLines 230-244 230 # No client tool results to feed back (e.g. server-only tool
231 # batch with no client tools to approve). On graceful stop just
232 # disable — there's nothing to surface for approval.
233 if self.stop_requested:
! 234 self.status = AutoRunStatus.USER_STOPPED
! 235 return
236 # Same drain-before-idle check applies.
237 injected = self._drain()
238 if injected:
! 239 body = self._continue_with_user_messages(body, injected)
! 240 continue
241 self.idle_reason = "done"
242 self.status = AutoRunStatus.IDLE
243 returnLines 251-260 251 # Graceful stop after a fully auto-executed round: don't start a new
252 # round. The tool results were fed back so the trace stays clean;
253 # disable auto mode (the supervisor publishes auto-mode-off).
254 if self.stop_requested:
! 255 self.status = AutoRunStatus.USER_STOPPED
! 256 return
257 # Inject any messages queued during this round alongside the tool
258 # results so the backend sees both on the next turn (§13.2).
259 injected = self._drain()
260 if injected:Lines 340-348 340 # runner is only ever fed via that enqueue path. Echoing again here would
341 # render the injected message twice to all observers, so drain only takes
342 # the queued messages and does NOT re-echo (CR Moderate 1).
343 if self._drain_inbound is None:
! 344 return []
345 return self._drain_inbound()
346
347 @staticmethod
348 def _append_user_messages(app/desktop/studio_server/chat/routes.pyLines 199-207 199 if detailed.status_code == HTTPStatus.OK and isinstance(detailed.parsed, list):
200 items: list[ChatSessionListItem] = []
201 for item in detailed.parsed:
202 if not isinstance(item, ApiSessionListItem):
! 203 continue
204 # Server-side join against the in-memory auto-run registry so the
205 # UI gets a single, point-in-time view of which sessions are
206 # actively running in auto mode (no two-list correlation client
207 # side). A sub-ms race here is self-healing on the next refresh.app/desktop/studio_server/chat/stream_session.pyLines 181-190 181 try:
182 parsed = json.loads(error_body)
183 detail = parsed.get("message", detail) or detail
184 code = parsed.get("code")
! 185 except json.JSONDecodeError:
! 186 pass
187 error_payload: dict[str, Any] = {
188 "type": "error",
189 "message": detail,
190 }Lines 451-459 451 """Clear the conversation's auto-mode flag for an intercepted
452 disable_auto_mode call. Imported lazily to avoid a circular import
453 (the auto registry depends on this module's round mechanics)."""
454 if not trace_id:
! 455 return
456 from app.desktop.studio_server.chat.auto.registry import auto_chat_registry
457
458 await auto_chat_registry.disable_for_trace(trace_id)app/desktop/studio_server/jobs/events.pyLines 71-79 71 except asyncio.TimeoutError:
72 # Cancels only the throwaway get() above; the feeder (and thus
73 # the subscription) is untouched and keeps draining.
74 yield KEEPALIVE_PING
! 75 continue
76 if item is None:
77 break
78 yield item
79 finally:
|
![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 implements an autonomous "Auto-Mode" feature for the assistant, allowing unattended multi-step tool execution that persists server-side. It introduces a server-side auto-run engine, registry, and event bus, alongside new built-in tools for enabling and disabling auto-mode. The web UI is updated with an auto-run store, a consent dialog, and footer controls, and is integrated with the chat session store to support message injection and session resyncing on load. Feedback on the changes includes guarding against a potential race condition in resyncOnLoad when switching sessions during asynchronous operations, and using a generic TypeVar in iter_with_keepalive to resolve type checking mismatches when reusing the utility with byte streams.
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.
![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: 11
🧹 Nitpick comments (3)
app/desktop/studio_server/chat/auto/test_api.py (1)
349-368: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReplace fixed sleeps with state-based waits in these async tests.
These
asyncio.sleep(...)calls make the assertions timing-sensitive; on slower CI the run may not have reached the expected state yet, which will produce intermittent failures. Reuse the existing polling pattern (_wait_settled) or add a small_wait_status(...)helper so the tests block onRUNNING/off conditions instead of scheduler timing.Also applies to: 819-834
🤖 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/test_api.py` around lines 349 - 368, Replace the fixed asyncio.sleep calls in this async test flow with state-based waiting so the assertions do not depend on scheduler timing. In the test around the run registry/drain_task flow, wait for the desired run state using the existing polling helpers such as _wait_settled or a small _wait_status helper keyed off run.record.status instead of sleeping before checking RUNNING or terminal/off behavior. Keep the stop request and terminal wait logic in sync with the run status transitions so the test blocks until the actual condition is reached.app/web_ui/src/routes/(app)/assistant/chat.svelte (1)
775-776: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse Tailwind motion utilities instead of custom pulse CSS.
animate-pulse motion-reduce:animate-nonekeeps the new animation styling in Tailwind and lets you remove the component-level keyframes. As per coding guidelines,app/web_ui/**/*.{svelte,css}should use Tailwind CSS for styling in the web UI.Proposed refactor
- <span class:auto-pulse={$autoModeWorking} aria-hidden="true" + <span + class={$autoModeWorking + ? "animate-pulse motion-reduce:animate-none" + : ""} + aria-hidden="true" >⏵⏵</span >- .auto-pulse { - animation: auto-pulse 1.4s ease-in-out infinite; - } - - `@keyframes` auto-pulse { - 0%, - 100% { - opacity: 1; - } - 50% { - opacity: 0.35; - } - } - - `@media` (prefers-reduced-motion: reduce) { - .auto-pulse { - animation: none; - } - }Also applies to: 835-853
🤖 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 775 - 776, The auto-mode indicator in the chat UI is still using the custom auto-pulse class, so update the affected markup in chat.svelte to use Tailwind motion utilities instead. Replace the component-level pulse styling around the span tied to $autoModeWorking with animate-pulse and motion-reduce:animate-none, and remove any related custom keyframes or CSS referenced by the same auto-pulse behavior in the assistant chat component.Source: Coding guidelines
app/web_ui/src/routes/(app)/assistant/chat_history_row.svelte (1)
35-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer Tailwind utilities over a local
<style>block for the status dot.This animation can live on the element class list, so the extra scoped CSS is avoidable here.
Suggested refactor
- <span - class="auto-dot size-2 shrink-0 rounded-full bg-primary" + <span + class="size-2 shrink-0 rounded-full bg-primary animate-pulse motion-reduce:animate-none" title="Auto mode is on" aria-label="Auto mode on" role="img" ></span> ... -<style> - .auto-dot { - animation: auto-dot-pulse 1.6s ease-in-out infinite; - } - - `@keyframes` auto-dot-pulse { - 0%, - 100% { - opacity: 1; - } - 50% { - opacity: 0.4; - } - } - - `@media` (prefers-reduced-motion: reduce) { - .auto-dot { - animation: none; - } - } -</style>As per coding guidelines,
app/web_ui/**/*.{svelte,css}: Use Tailwind CSS for styling in the web UI.Also applies to: 74-93
🤖 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 35 - 39, The status dot in chat_history_row.svelte is using a local scoped style when it should be expressed with Tailwind utilities on the element itself. Update the span used for the auto-mode indicator and remove the corresponding local CSS by moving the animation/styling into the class list, keeping the existing status-dot behavior intact while avoiding the extra <style> block.Source: Coding guidelines
🤖 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/api.py`:
- Around line 30-46: The auto-enable and decline request models are trusting
browser-supplied internal state, which lets clients smuggle or spoof tool-call
execution. Update `EnableAutoRequest` and `DeclineAutoRequest` to carry only
identifiers, then in `enable_auto_mode()` and the decline handler
rebuild/validate `AutoChatSeed` and sibling tool calls from trusted server-side
state for the given `trace_id` and `enable_tool_call_id`. Ensure
`auto_chat_registry.start()` receives only validated pending tool calls, and
only generate synthetic tool outputs from tool-call IDs that match the pending
upstream turn.
In `@app/desktop/studio_server/chat/auto/events.py`:
- Around line 27-30: The live subscription shutdown path is missing the terminal
close signal, so already-attached subscribers keep waiting after auto-mode-off.
Update the auto-mode-off/off-marker emission in the relevant events flow so it
passes close=True when publishing the terminal transition, and ensure the
subscriber loop tied to _ByteSubscriber exits on that signal instead of
continuing to keep the /events SSE connection alive.
In `@app/desktop/studio_server/chat/auto/models.py`:
- Around line 52-64: The InboundMessage schema currently allows callers to
override role, which lets /message accept non-user chat roles. Restrict this
contract in InboundMessage so the role is fixed to user rather than
user-provided, and keep as_chat_message aligned with that assumption to ensure
auto-mode only ingests user messages.
In `@app/desktop/studio_server/chat/auto/registry.py`:
- Around line 156-167: The current emit() logic in registry.py clears the entire
replay buffer whenever _extract_trace_id(payload) matches anywhere in a chunk,
which drops later SSE events that arrive after the trace in the same payload.
Update emit() to handle multi-event payloads by splitting on SSE frame
boundaries, publishing each frame in order, and clearing only the buffered bytes
up to the trace frame while preserving any post-trace events in self.buffer for
late reattach replay.
In `@app/desktop/studio_server/chat/stream_session.py`:
- Around line 353-355: The auto-mode cleanup in stream_session should use the
known trace fallback instead of only round_state.trace_id. Update the
disable-event branch around _clear_auto_mode_flag in stream_session.py to pass
trace_id_for_error when available, falling back to round_state.trace_id, so the
flag is cleared even when no fresh trace event was emitted. Keep the change
localized to the disable_evt handling path and preserve the existing non_disable
filtering logic.
In `@app/web_ui/src/lib/chat/auto_run_store.ts`:
- Around line 459-497: The optimistic working state in sendMessage is not
cleared on request failure, so the indicator can stay stuck on after an inject
error. Update sendMessage in auto_run_store to reset setWorking(false) in both
the fetch catch path and the !response.ok path before returning the error
result, so failed sends don’t leave the auto-mode UI in a busy state.
In `@app/web_ui/src/lib/chat/chat_session_store.ts`:
- Around line 496-500: The trace selection in handleAutoModeConsent is dropping
consent-required events when payload.traceId is null because it only falls back
to continuationTraceId. Update this logic to prefer the persisted assistant
trace from the current session state first, using the trace stored by
setLastAssistantTraceId() on the last assistant message, and only then fall back
to continuationTraceId. Keep the fix localized to chat_session_store.ts and the
handleAutoModeConsent path so nullable AutoModeConsentRequiredPayload.traceId is
still handled correctly in live chat.
In `@app/web_ui/src/routes/`(app)/assistant/chat_history_row.svelte:
- Around line 57-70: The delete action in chat_history_row.svelte is only
revealed via group-hover, so keyboard users can focus the row without seeing it.
Update the same menu container around TableActionMenu to also become visible on
focus (for example by adding a group-focus/focus-within path alongside the
existing hover behavior) while preserving the click stopPropagation and
onDelete(row.id) wiring.
In `@app/web_ui/src/routes/`(app)/assistant/chat.svelte:
- Around line 53-79: Keep the auto-mode enable flow pending until
requestEnable() finishes. In openManualAutoMode, consentPending is currently
cleared in the finally block right after consentDialog.prompt() returns, which
lets the UI re-enable before auto_run_store.requestEnable({ trace_id }) settles
and can trigger duplicate enables. Move the consentPending reset so it stays
true through the requestEnable path and is only cleared after the server request
completes or fails, preserving the guard on the assistant/chat.svelte flow.
In `@specs/projects/assistant_auto_mode/architecture.md`:
- Around line 76-83: The per-run auto stream contract still uses auto-mode-off
for burst-ending states, but it should be updated to auto-mode-idle for those
cases instead. In the architecture spec section describing GET
/api/chat/auto/{run_id}/events, revise the event list and the state/reason
mapping so assistant-asks, done, error, and max_rounds no longer emit
auto-mode-off; keep auto-mode-on for activation and use auto-mode-idle for
idle/burst end behavior, matching the StreamEventProcessor and auto-mode event
vocabulary consistently.
In `@specs/projects/assistant_auto_mode/functional_spec.md`:
- Around line 62-63: Update the auto-mode consent and lifecycle wording in the
functional spec to match the conversation-scoped contract: the summary near the
auto-mode trigger description and the dialog copy in the later sections should
no longer say consent is required every time or that auto-mode turns off
automatically when the assistant asks or finishes. Align the affected text in
the auto-mode overview and the related dialog/flow sections so they consistently
state that auto-mode remains active until the user explicitly stops it.
---
Nitpick comments:
In `@app/desktop/studio_server/chat/auto/test_api.py`:
- Around line 349-368: Replace the fixed asyncio.sleep calls in this async test
flow with state-based waiting so the assertions do not depend on scheduler
timing. In the test around the run registry/drain_task flow, wait for the
desired run state using the existing polling helpers such as _wait_settled or a
small _wait_status helper keyed off run.record.status instead of sleeping before
checking RUNNING or terminal/off behavior. Keep the stop request and terminal
wait logic in sync with the run status transitions so the test blocks until the
actual condition is reached.
In `@app/web_ui/src/routes/`(app)/assistant/chat_history_row.svelte:
- Around line 35-39: The status dot in chat_history_row.svelte is using a local
scoped style when it should be expressed with Tailwind utilities on the element
itself. Update the span used for the auto-mode indicator and remove the
corresponding local CSS by moving the animation/styling into the class list,
keeping the existing status-dot behavior intact while avoiding the extra <style>
block.
In `@app/web_ui/src/routes/`(app)/assistant/chat.svelte:
- Around line 775-776: The auto-mode indicator in the chat UI is still using the
custom auto-pulse class, so update the affected markup in chat.svelte to use
Tailwind motion utilities instead. Replace the component-level pulse styling
around the span tied to $autoModeWorking with animate-pulse and
motion-reduce:animate-none, and remove any related custom keyframes or CSS
referenced by the same auto-pulse behavior in the assistant chat component.
🪄 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: ab33f9ef-6ac6-495a-99b2-ed396499bad3
📒 Files selected for processing (68)
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/events.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/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/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
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
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/desktop/studio_server/jobs/registry.py`:
- Around line 553-563: `wait_many` currently re-reads `self._jobs[job_id]` after
awaiting, so a concurrent `delete()` can remove a terminal job and trigger a
`KeyError`. Update `Registry.wait_many` to mirror `wait()` by capturing the job
records during the initial validation loop (alongside `job =
self._require(job_id)`), then return those captured records after
`asyncio.wait_for(asyncio.gather(...))` instead of indexing `self._jobs` again.
🪄 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: 86050c58-21bb-4265-9161-9b07482a763a
📒 Files selected for processing (9)
app/desktop/studio_server/jobs/api.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/core/kiln_ai/adapters/eval/eval_runner.pylibs/server/kiln_server/utils/agent_checks/annotations/get_api_jobs_wait.json
🚧 Files skipped from review as they are similar to previous changes (1)
- app/web_ui/src/lib/api_schema.d.ts
Comment on lines
+553
to
+563
| pending_events: list[asyncio.Event] = [] | ||
| for job_id in job_ids: | ||
| job = self._require(job_id) | ||
| ev = self._completion_events.setdefault(job_id, asyncio.Event()) | ||
| if not job.status.is_terminal: | ||
| pending_events.append(ev) | ||
| if pending_events: | ||
| await asyncio.wait_for( | ||
| asyncio.gather(*(ev.wait() for ev in pending_events)), timeout | ||
| ) | ||
| return [self._jobs[job_id] for job_id in job_ids] |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
wait_many can raise KeyError if a waited job is deleted concurrently.
Unlike wait() (line 528/536), which returns the job reference captured up front, wait_many re-reads self._jobs[job_id] after awaiting. delete() is permitted on terminal jobs, so a concurrent delete of a job that just reached a terminal state can pop it from self._jobs at an await point during/after the gather, turning the final comprehension into an unhandled KeyError (→ 500). Capture the records during the up-front validation pass to mirror wait()'s immunity.
🛡️ Proposed fix: capture records up front
pending_events: list[asyncio.Event] = []
+ jobs: list[JobRecord] = []
for job_id in job_ids:
job = self._require(job_id)
+ jobs.append(job)
ev = self._completion_events.setdefault(job_id, asyncio.Event())
if not job.status.is_terminal:
pending_events.append(ev)
if pending_events:
await asyncio.wait_for(
asyncio.gather(*(ev.wait() for ev in pending_events)), timeout
)
- return [self._jobs[job_id] for job_id in job_ids]
+ return jobs📝 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
| pending_events: list[asyncio.Event] = [] | |
| for job_id in job_ids: | |
| job = self._require(job_id) | |
| ev = self._completion_events.setdefault(job_id, asyncio.Event()) | |
| if not job.status.is_terminal: | |
| pending_events.append(ev) | |
| if pending_events: | |
| await asyncio.wait_for( | |
| asyncio.gather(*(ev.wait() for ev in pending_events)), timeout | |
| ) | |
| return [self._jobs[job_id] for job_id in job_ids] | |
| pending_events: list[asyncio.Event] = [] | |
| jobs: list[JobRecord] = [] | |
| for job_id in job_ids: | |
| job = self._require(job_id) | |
| jobs.append(job) | |
| ev = self._completion_events.setdefault(job_id, asyncio.Event()) | |
| if not job.status.is_terminal: | |
| pending_events.append(ev) | |
| if pending_events: | |
| await asyncio.wait_for( | |
| asyncio.gather(*(ev.wait() for ev in pending_events)), timeout | |
| ) | |
| return jobs |
🤖 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/registry.py` around lines 553 - 563,
`wait_many` currently re-reads `self._jobs[job_id]` after awaiting, so a
concurrent `delete()` can remove a terminal job and trigger a `KeyError`. Update
`Registry.wait_many` to mirror `wait()` by capturing the job records during the
initial validation loop (alongside `job = self._require(job_id)`), then return
those captured records after `asyncio.wait_for(asyncio.gather(...))` instead of
indexing `self._jobs` again.
Collaborator
Author
|
Folded this branch down into #1517 rather than merging through GitHub. This PR was stacked directly on top of
GitHub auto-detected those commits in #1517's branch and marked this PR as merged. #1517 is now the single consolidated PR (retitled and its description rewritten to cover eval-as-a-job, the jobs-API improvements, and auto mode). No code was lost — review and follow-up tracking continue on #1517. |
2 tasks
This was referenced Jul 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rescopes the assistant auto mode prototype (originally #1451) to target
leonard/kil-686-eval-job(#1517) instead of the eval/finetune/RAG-via-job refactor stack (#1436 → #1450), which we're dropping.What
Auto mode lets the assistant run steps automatically (reflective optimization) without per-step approval, with explicit consent and a stop control.
chat/auto/subsystem: auto-run registry, runner, event bus, SSE, and API (/api/chat/auto/*: enable, decline, resolve, sessions,{run_id}message/stop/events).enable_auto_mode/disable_auto_modebuilt-in tools + tool-registry wiring.auto_run_store, consent dialog, chat-history grouping, and a reworkedchat.svelte(in-transcript live working/idle, inject-on-send, reconnect handling).specs/projects/assistant_auto_mode/.How it was rescoped
The auto-mode work was almost entirely independent of the dropped stack. Mechanically, this branch is the net auto-mode diff applied onto
leonard/kil-686-eval-job:jobs/events.py; it's ported here (KeepalivePing/KEEPALIVE_PING/iter_with_keepalive) sochat/autoreuses it unchanged. No eval/finetune/RAG-via-job code is included.chat.svelte— took auto-mode's version (the built+tested feature) and re-applied the three base UI refactors that landed since the fork: scrollbar-to-the-side, DaisyUIbtn-circlesend/stop, and the centering-wrapper layout..env.example— keptPUBLIC_ENABLE_JOBS; droppedPUBLIC_SHOW_TOOL_CALL_DETAILS(auto-mode removed that debug feature).History is flattened into one commit; the phased narrative is preserved in
specs/projects/assistant_auto_mode/phase_plans/.Testing
uv run ./checks.sh --agent-mode→ passes (Python lint/format/type/tests, web lint/format/check/test/build, OpenAPI schema in sync).Note
Supersedes #1451 (which targeted the dropped
leonard/kil-687-rag-via-job). #1451 can be closed.🤖 Generated with Claude Code