Repository navigation
editor: streaming, live work motion, inbox sidebar, approvals, context meter - #502
Conversation
Formatting only; no behaviour change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Grok Build's listing now recognises the 1.0.4x "Available models:" header and its `(default)` marker, and a signed-out CLI is reported as such instead of as an engine with no models. The Forge drops unauthenticated engines from the runnable set, the catalog policy can tell whether an engine reported any enabled model, and the picker greys out and disables the tabs of engines that cannot offer one, preferring a usable tab. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Approval observations carry their recorded decision into the scene, and the gate remembers a decision the engine acknowledged, so a card reads as allowed or denied as soon as the receipt lands instead of staying in flight until the resolution arrives. Gates are keyed by the engine's approval identity, the name both the gesture and its receipt carry. An approval or question pauses a work section rather than splitting it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The thread route paints a compact context meter at the conversation end of the titlebar, just left of the inspector rule or the caption controls, showing the same validated reading as the composer's ring. The inspector rule now continues up through the titlebar like the sidebar's, and the header's title keeps clear of the meter. A shared material face (`flame`) backs the meter's details card. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`stable_reveal_end` finds the longest prefix of a streaming body whose rendering can no longer change: it holds back the trailing word, an unclosed inline construct, unterminated fences and tables, a trailing run of pipe lines, and marker-only lines. GPUI wraps greedily in one pass, so painting only that prefix never moves a glyph already on screen; the tests check it for every prefix of sample replies. `RevealSteps` walks a prefix one word, inline construct, or block at a time, and the renderer fades fresh segments through `RevealFade` without touching layout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A streaming reply suppresses the turn's status block, which carried the only clock basis, so the section header fell back to a bare "Working" while the summary streamed. The basis now also lives on the turn scene, and the section title counts from it until the turn ends. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replies and the prose inside a turn's work now stream in: each row shows only the stable prefix of its body (see `markdown_reveal`), revealing everything that became stable at most every 150 ms as one segment that fades in at a single opacity. Fences and tables appear only complete, and nothing on screen ever moves. While a turn is live its work moves too: tool chains, chain steps, and the status line enter with the texts-reveal recipe, the status line leaves by fading while its height closes, and chain headers and the elapsed header count like an odometer, appended clauses rising in. The work section folds the moment the reply's first segment shows. Settled history and a row's first render paint at rest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The sidebar reads as an inbox: only threads with something unseen (a finished or failed run, an open question or approval) show outright. Running threads collapse into a Working group and read idle ones into History, which keeps its age groups inside. Group headers use the age label style with a count and a chevron; opening one flips the chevron and fades its rows in. An unread thread stays at the top while open. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A one-device-pixel rule runs edge to edge under the Context rows, the column inset below them, with the shell's crosshair where it meets the inspector's left rule, so the column reads as a grid. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Moves the gpui-ce pin to 8d8c0734, which adds `Styled::transform`: translate and uniform scale about an origin, applied at paint and hit testing without touching layout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The profile menu takes its selector's exact width and follows the transitions-dev dropdown: it grows from 97% about its bottom-left corner while fading in over 250 ms, and settles to 99% while fading out over 150 ms. The picker menu animation gains an entrance and separate open and close durations; the model picker keeps its 100 ms slide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
37 issues found across 103 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/ui/src/markdown_renderer.rs">
<violation number="1" location="modules/ui/src/markdown_renderer.rs:780">
P2: Inline link favicons and hoisted citation pills bypass this fade, so newly revealed links can pop in at full opacity while their text fades. Apply the segment opacity to those adornments too.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/tests/work_group_header.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/tests/work_group_header.rs:560">
P3: This fixture marks the approval as already approved, so it does not represent the pending approval described in the test. Set `decision` to `None` to exercise the waiting state.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/reply_reveal.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/reply_reveal.rs:100">
P2: A previously revealed line can become a table when later chunks add `|`; because the body still starts with `revealed`, this code ignores the regressed stable cutoff and later reparses the prose as a table, shifting existing text. Hold table-capable lines out of the reveal until their structure is stable.</violation>
</file>
<file name="modules/frontend/src/desktop_shell.rs">
<violation number="1" location="modules/frontend/src/desktop_shell.rs:299">
P2: `content_inset` stays at 24 px when the meter is absent, so long thread titles can cross the inspector divider still drawn at `window - inspector_width`. Reserve the divider's gap in the no-meter case too.</violation>
</file>
<file name="modules/backend/src/project_subtitles.rs">
<violation number="1" location="modules/backend/src/project_subtitles.rs:269">
P2: A transient Git read error clears the cached image: `.ok()` makes `observation` `None`, this sets the identity to `None`, and `record` replaces the icon with the default. Preserve the previous identity on `None` observations so temporary read failures do not blank project artwork.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/turn_row.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/turn_row.rs:447">
P2: This treats the first assistant block, including commentary, as the promoted reply. In legacy turns, commentary can precede later work or the actual reply, so work groups fold prematurely; select the non-commentary promoted/latest reply instead.</violation>
</file>
<file name="modules/backend/src/project_icon_service.rs">
<violation number="1" location="modules/backend/src/project_icon_service.rs:63">
P2: On Unix, a FIFO at any searched icon path makes `File::open` wait indefinitely, so resolution never reaches the remote fallback and the cache remains pending. Reject non-regular files or open these paths nonblocking.</violation>
<violation number="2" location="modules/backend/src/project_icon_service.rs:102">
P1: `Policy::limited(3)` caps hops but still follows cross-host HTTP(S) redirects; `download` checks only the initial URL, so an untrusted GitLab/Gitea endpoint can make Forge request loopback or private services. Validate each redirect target or disable redirects for metadata requests.</violation>
<violation number="3" location="modules/backend/src/project_icon_service.rs:165">
P3: GitHub icon metadata comes from the unauthenticated `api.github.com/users/{owner}` endpoint, which is rate-limited to 60 requests/hour per IP. Once a few icon-less projects are attached (each retried every 5 minutes per the 300s freshness in `refresh_icon`), the endpoint starts returning 403, the png quietly never loads, and every subsequent retry hits the same limit. Consider treating a 403/rate-limit response as a reason to skip retrying for much longer (or cache the failure), or falling back to the unauthenticated-repo HTML icon if the product can tolerate it.</violation>
</file>
<file name="modules/tracing/src/export.rs">
<violation number="1" location="modules/tracing/src/export.rs:28">
P3: `thread_name` metadata events are emitted twice in every snapshot. `History::push` stores each `M`/`thread_name` event in both `history.events` and `history.threads`, and `Snapshot::new` clones `history.events` (which already contains them) and then appends `history.threads.values()` again. Deduplicate so each thread name is written once, e.g. only extend for tids not already present in `events`.</violation>
<violation number="2" location="modules/tracing/src/export.rs:42">
P3: Operations that ran longer than the 120 s window and completed before the snapshot disappear from the capture entirely. Their `b` was pruned from `history.events` (and removed from `history.open` when `e` arrived), so `Snapshot::new` neither re-anchors a truncated begin (that only happens for entries still in `open`) nor keeps the end: this retain drops the orphaned `e`. The longest operations — exactly the ones this recorder exists to diagnose — leave no trace. When `e` arrives, re-insert a truncated begin (like the `begin_truncated` path) if the matching `b` has already aged out of the window.</violation>
<violation number="3" location="modules/tracing/src/export.rs:126">
P3: A failure to trim old files makes a successful snapshot report as an export failure. `retain_recent` runs after `fs::rename` already committed the new capture; if it errors (e.g. permission or locked file on Windows), `write_snapshot` returns `Err` and `writer` prints "Artisan trace export failed" even though the trace was saved. Log and continue so retention problems cannot mask a successful capture.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/render_blocks.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/render_blocks.rs:914">
P2: The chain and its first activity step both receive an entrance, compounding the 12 px rise, 3 px blur, and opacity. Skip one wrapper for the first step.</violation>
</file>
<file name="modules/ui/src/markdown_reveal.rs">
<violation number="1" location="modules/ui/src/markdown_reveal.rs:323">
P2: This treats every `\*` as escaped, but an even-length backslash run leaves `*` active. A later closer can therefore restyle text already revealed by the streaming cutoff; determine escaping from the parity of the preceding backslash run.</violation>
<violation number="2" location="modules/ui/src/markdown_reveal.rs:390">
P2: CRLF-terminated fences are treated as open because `closing_end` points at `\r`, not `\n`. Accept CRLF (and lone CR if supported) as a line terminator so the completed fence can be revealed while the reply streams.</violation>
</file>
<file name="modules/tracing/src/recorder.rs">
<violation number="1" location="modules/tracing/src/recorder.rs:374">
P2: A manual save can omit already-queued trace events when the backlog exceeds 1,024, because `Control::Save` is processed before the rest of the separate event queue. Order the save as a barrier after all events submitted before the request.</violation>
<violation number="2" location="modules/tracing/src/recorder.rs:432">
P3: A manual `save()` can silently produce no file. `save()` only signals that the `Control::Save` was accepted; if the two-slot writer channel is full when the worker drains the control (e.g. another snapshot still being written), `try_send` fails and the request is dropped with just a bumped `dropped` counter. On Ctrl+Shift+F11 the user gets neither a file nor an error. Either reserve capacity or surface the failure (e.g. have `save()` report that the write was deferred/dropped).</violation>
</file>
<file name="modules/frontend/Cargo.toml">
<violation number="1" location="modules/frontend/Cargo.toml:25">
P3: `artisan-tracing` is a non-optional dependency, so every production payload (which never enables `flight-recorder`) still compiles and links the crate, even though all of its code is `#[cfg(feature = "enabled")]` and every consumer is gated behind `#[cfg(feature = "flight-recorder")]` (the `flight_recorder` module, `app_entry.rs`, `impl_render.rs`, `history_paging.rs`, etc.). Declare it `optional = true` and enable it only with the feature: `flight-recorder = ["dep:artisan-tracing", "artisan-tracing/enabled"]` — unless any call site intentionally relies on the crate's erasing macros with no cfg gate, in which case the crate must stay non-optional and this note stands as-is. The same pattern applies to `modules/backend/Cargo.toml`.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/work_motion.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/work_motion.rs:141">
P2: `end_frame` drops every reveal whose row did not render this frame. A work group folds while its turn is still live (`folded_for_reply` in render_blocks.rs), so a streaming `DetailRow::Assistant` inside it stops rendering and its reveal is silently discarded; when the group renders again (e.g., the reply is replaced and `reply_shown` flips back to false, or the group re-opens), `reveal` re-creates the state with `ReplyReveal::begin` and the prose replays its full paced fade from the start, including a body already shown. Keep reveals keyed by `SceneId` across folded frames, or resume from the last revealed prefix instead of `begin`.</violation>
<violation number="2" location="modules/frontend/src/conversation_surface/work_motion.rs:206">
P3: `entrances` and `phrases` grow for the whole lifetime of the row: every work row ever painted adds a `entrances` entry keyed by `scene_id`/`chain:…`, and every label/clause change adds a `phrases` entry keyed by selector suffix. A long-running turn with hundreds of tool rows retains all of them, and the only removal anywhere is the `"status"` entrance key. Prune entries whose scene ids or selectors are no longer rendered (mirroring the `reveals` cleanup in `end_frame`).</violation>
<violation number="3" location="modules/frontend/src/conversation_surface/work_motion.rs:310">
P2: Article-to-count changes roll the literal article instead of the value it represents, producing `a`/`n` → `2` rather than `1` → `2`. Normalize `a` and `an` to `1` when setting the roll source.</violation>
</file>
<file name="modules/frontend/src/native_application/impl_sidebar_threads.rs">
<violation number="1" location="modules/frontend/src/native_application/impl_sidebar_threads.rs:388">
P3: `schedule_regroup` still scans the whole listing with `next_regrouping`, but only `History` rows are age-grouped now that `section_recent_threads` splits the sidebar into sections. An Unread or Working row crossing the 24h/3d/30d boundary still schedules a timer and repaints the sidebar with no visible change. Restrict `next_regrouping` to the rows that end up in `History` (or accept the extra repaint deliberately).</violation>
<violation number="2" location="modules/frontend/src/native_application/impl_sidebar_threads.rs:489">
P3: A group holding one row announces "Working, 1 threads". Pluralize with the count: `thread` for 1, `threads` otherwise.</violation>
<violation number="3" location="modules/frontend/src/native_application/impl_sidebar_threads.rs:527">
P3: Modified Enter/Space also toggles this disclosure and stops propagation; ignore modified keystrokes when handling button activation.</violation>
</file>
<file name="tests/tracing/recorder.rs">
<violation number="1" location="tests/tracing/recorder.rs:65">
P2: This fixed 750 ms window can resume the heartbeat before the watchdog observes a stall, so the automatic-capture assertions fail nondeterministically under scheduler delays. Keep the heartbeat stale longer or synchronize the test with the observed stall before resuming it.</violation>
</file>
<file name="modules/frontend/src/native_model_selector/interaction.rs">
<violation number="1" location="modules/frontend/src/native_model_selector/interaction.rs:96">
P3: This insertion leaves the existing save/error/authority rustdoc attached to `set_blocked_engines`, which only changes sign-in-blocked engines, and leaves `set_status` undocumented. Move that comment back to `set_status` and give this setter its own description.</violation>
</file>
<file name="modules/tracing/src/watchdog.rs">
<violation number="1" location="modules/tracing/src/watchdog.rs:23">
P3: If the watchdog thread fails to spawn, monitoring silently disappears while `Heartbeat` still behaves as if active and `tick()` keeps updating the stamp. The stall/resume detection this API advertises never runs and nothing reports the failure. Match the recorder: log a message when spawn fails.</violation>
<violation number="2" location="modules/tracing/src/watchdog.rs:36">
P2: This branch leaves `stamp` at the pre-pause tick, so the next poll can report the suspended interval as `heartbeat.stalled` and trigger a false incident. Reset the heartbeat baseline on resume, while preserving the unarmed sentinel.</violation>
</file>
<file name="modules/frontend/src/native_transport_service/flight_recorder.rs">
<violation number="1" location="modules/frontend/src/native_transport_service/flight_recorder.rs:31">
P3: `AcknowledgePatch` carries a `thread_id`, but this match omits it, so queue and execution spans cannot attribute acknowledgements to a conversation. Include it alongside `RequestSnapshot`.</violation>
</file>
<file name="docs/runbooks/project-icons.md">
<violation number="1" location="docs/runbooks/project-icons.md:17">
P3: This says private repositories fall back to the logo, but GitHub can return the owner's public avatar even when the repository cannot be read. Clarify that GitHub may still show that avatar.</violation>
</file>
<file name="modules/ui/src/markdown_reveal_tests.rs">
<violation number="1" location="modules/ui/src/markdown_reveal_tests.rs:196">
P2: This assertion compares parsed characters, styles, and block paths, but it never checks rendered line positions. A regression that moves revealed text to a different wrapped line while preserving its Markdown structure still passes; include rendered line positions or narrow the test's no-shift claim.</violation>
</file>
<file name="docs/runbooks/flight-recorder.md">
<violation number="1" location="docs/runbooks/flight-recorder.md:12">
P2: The writer uses `std::env::temp_dir()`, which prefers `TMP` over `TEMP` on Windows, so `%TEMP%` is not always the capture directory. Describe the process temp directory instead.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/render_sections.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/render_sections.rs:132">
P2: This key drops the run scope, so approvals in different runs that reuse a provider ID share one gate. Settling one receipt then hides the other still-open approval; key the gate and receipt correlation by `(run_id, approval_id)`.</violation>
</file>
<file name="modules/frontend/src/native_transport_service/service_lifecycle.rs">
<violation number="1" location="modules/frontend/src/native_transport_service/service_lifecycle.rs:248">
P2: This samples `command.depth` only before `try_send` and never when the receiver dequeues a command, so traces undercount successful sends and retain a nonzero depth after the queue drains. Emit the depth after successful enqueue and again on each dequeue.</violation>
</file>
<file name="scripts/trace.py">
<violation number="1" location="scripts/trace.py:85">
P3: `merge` discards every source capture's metadata, so the merged file keeps only `sources`. Running `summary` on the merged output then reports `unknown capture: unknown` and `Dropped: 0; evicted: 0` even when the inputs carried real `dropped_records`/`evicted_records` counts, and `session_start_us`/`role` are gone — the same fields `merge` uses to dedupe process ids are lost if the merged file is merged again, which reassigns different pids to the same process and breaks the stated deduplication. Carry the union of the inputs' metadata into the output instead of dropping it.</violation>
</file>
<file name="modules/frontend/src/native_model_selector/state.rs">
<violation number="1" location="modules/frontend/src/native_model_selector/state.rs:435">
P2: `set_blocked_engines` can swap the active engine and clear the search query while the picker popover is open: the verdict change arrives in `NativeApplication::render`, which calls `sync_model_selector_blocked_engines` on every frame with no `self.open` guard. The menu then silently jumps to another engine and erases the user's query mid-interaction, contradicting the stated contract in `set_snapshot` that runtime readiness is not consulted while the picker is open. Guard the tab move (and query/axis reset) with `!self.open`, or close the popover on a verdict change.</violation>
</file>
<file name="modules/frontend/src/native_model_selector/render/menu.rs">
<violation number="1" location="modules/frontend/src/native_model_selector/render/menu.rs:293">
P3: A greyed engine's tab is still exposed as `role=Button` with a plain `aria_label`, but has no action and no `aria_disabled`. Screen readers announce an actionable button that does nothing when focused or activated. Set `aria_disabled` (and keep the "(unavailable)" suffix in the label if desired).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| .get_or_init(|| { | ||
| reqwest::Client::builder() | ||
| .timeout(Duration::from_secs(3)) | ||
| .redirect(reqwest::redirect::Policy::limited(3)) |
There was a problem hiding this comment.
P1: Policy::limited(3) caps hops but still follows cross-host HTTP(S) redirects; download checks only the initial URL, so an untrusted GitLab/Gitea endpoint can make Forge request loopback or private services. Validate each redirect target or disable redirects for metadata requests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/backend/src/project_icon_service.rs, line 102:
<comment>`Policy::limited(3)` caps hops but still follows cross-host HTTP(S) redirects; `download` checks only the initial URL, so an untrusted GitLab/Gitea endpoint can make Forge request loopback or private services. Validate each redirect target or disable redirects for metadata requests.</comment>
<file context>
@@ -0,0 +1,212 @@
+ .get_or_init(|| {
+ reqwest::Client::builder()
+ .timeout(Duration::from_secs(3))
+ .redirect(reqwest::redirect::Policy::limited(3))
+ .user_agent("Artisan-Editor-project-icons")
+ .build()
</file context>
| ) -> AnyElement { | ||
| let theme = render.theme; | ||
| let (presentation, icons) = leaf.presentation(spans, render.titles, render.counters); | ||
| let fade = render.claim_leaf(presentation.text.len()); |
There was a problem hiding this comment.
P2: Inline link favicons and hoisted citation pills bypass this fade, so newly revealed links can pop in at full opacity while their text fades. Apply the segment opacity to those adornments too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/ui/src/markdown_renderer.rs, line 780:
<comment>Inline link favicons and hoisted citation pills bypass this fade, so newly revealed links can pop in at full opacity while their text fades. Apply the segment opacity to those adornments too.</comment>
<file context>
@@ -665,8 +777,9 @@ fn render_leaf(
) -> AnyElement {
let theme = render.theme;
let (presentation, icons) = leaf.presentation(spans, render.titles, render.counters);
+ let fade = render.claim_leaf(presentation.text.len());
if presentation.citations.is_empty() {
- return selectable_leaf(selector, &presentation, icons, theme);
</file context>
| self.marks.clear(); | ||
| self.resync = true; | ||
| } | ||
| let start = self.revealed.len(); |
There was a problem hiding this comment.
P2: A previously revealed line can become a table when later chunks add |; because the body still starts with revealed, this code ignores the regressed stable cutoff and later reparses the prose as a table, shifting existing text. Hold table-capable lines out of the reveal until their structure is stable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/reply_reveal.rs, line 100:
<comment>A previously revealed line can become a table when later chunks add `|`; because the body still starts with `revealed`, this code ignores the regressed stable cutoff and later reparses the prose as a table, shifting existing text. Hold table-capable lines out of the reveal until their structure is stable.</comment>
<file context>
@@ -0,0 +1,216 @@
+ self.marks.clear();
+ self.resync = true;
+ }
+ let start = self.revealed.len();
+ if start < stable {
+ match motion {
</file context>
| let content_inset = if context_meter.is_some() { | ||
| (meter_right + px(CONTEXT_METER_WIDTH_PX + 24.0) - controls_width).max(px(24.0)) | ||
| } else { | ||
| px(24.0) | ||
| }; |
There was a problem hiding this comment.
P2: content_inset stays at 24 px when the meter is absent, so long thread titles can cross the inspector divider still drawn at window - inspector_width. Reserve the divider's gap in the no-meter case too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/desktop_shell.rs, line 299:
<comment>`content_inset` stays at 24 px when the meter is absent, so long thread titles can cross the inspector divider still drawn at `window - inspector_width`. Reserve the divider's gap in the no-meter case too.</comment>
<file context>
@@ -277,6 +289,18 @@ pub fn desktop_shell(
+ let controls_width = px(DESKTOP_TITLEBAR_CONTROL_WIDTH_PX * 3.0);
+ let meter_right =
+ inspector_width.unwrap_or(px(0.0)).max(controls_width) + px(DESKTOP_CONTEXT_METER_GAP_PX);
+ let content_inset = if context_meter.is_some() {
+ (meter_right + px(CONTEXT_METER_WIDTH_PX + 24.0) - controls_width).max(px(24.0))
+ } else {
</file context>
| let content_inset = if context_meter.is_some() { | |
| (meter_right + px(CONTEXT_METER_WIDTH_PX + 24.0) - controls_width).max(px(24.0)) | |
| } else { | |
| px(24.0) | |
| }; | |
| let content_inset = if context_meter.is_some() { | |
| (meter_right + px(CONTEXT_METER_WIDTH_PX + 24.0) - controls_width).max(px(24.0)) | |
| } else { | |
| (inspector_width.unwrap_or(controls_width) - controls_width + px(24.0)).max(px(24.0)) | |
| }; |
| (Some(_), _) => 0, | ||
| (None, previous) => previous.map_or(0, |previous| previous.failures) + 1, | ||
| }; | ||
| let icon_identity = crate::project_icon_service::identity(observation.as_ref()); |
There was a problem hiding this comment.
P2: A transient Git read error clears the cached image: .ok() makes observation None, this sets the identity to None, and record replaces the icon with the default. Preserve the previous identity on None observations so temporary read failures do not blank project artwork.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/backend/src/project_subtitles.rs, line 269:
<comment>A transient Git read error clears the cached image: `.ok()` makes `observation` `None`, this sets the identity to `None`, and `record` replaces the icon with the default. Preserve the previous identity on `None` observations so temporary read failures do not blank project artwork.</comment>
<file context>
@@ -243,17 +266,74 @@ impl ProjectSubtitles {
(Some(_), _) => 0,
(None, previous) => previous.map_or(0, |previous| previous.failures) + 1,
};
+ let icon_identity = crate::project_icon_service::identity(observation.as_ref());
+ let retained = previous.filter(|previous| previous.icon_identity == icon_identity);
+ let icon = retained.map_or_else(
</file context>
| let icon_identity = crate::project_icon_service::identity(observation.as_ref()); | |
| let icon_identity = if observation.is_none() { | |
| previous.and_then(|previous| previous.icon_identity.clone()) | |
| } else { | |
| crate::project_icon_service::identity(observation.as_ref()) | |
| }; |
| its project-specific artwork. Private or inaccessible metadata requires no | ||
| login and falls back to the logo. No credentials are sent by this resolver. |
There was a problem hiding this comment.
P3: This says private repositories fall back to the logo, but GitHub can return the owner's public avatar even when the repository cannot be read. Clarify that GitHub may still show that avatar.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At docs/runbooks/project-icons.md, line 17:
<comment>This says private repositories fall back to the logo, but GitHub can return the owner's public avatar even when the repository cannot be read. Clarify that GitHub may still show that avatar.</comment>
<file context>
@@ -0,0 +1,36 @@
+GitHub resolves the public organization/user avatar. GitLab, Bitbucket,
+Codeberg, and Gitea resolve repository artwork before owner artwork. GitHub
+has no dedicated repository avatar endpoint, so a local project file provides
+its project-specific artwork. Private or inaccessible metadata requires no
+login and falls back to the logo. No credentials are sent by this resolver.
+
</file context>
| its project-specific artwork. Private or inaccessible metadata requires no | |
| login and falls back to the logo. No credentials are sent by this resolver. | |
| its project-specific artwork. GitHub may still resolve a public account avatar when a repository cannot be read; inaccessible metadata falls back to the logo. No credentials are | |
| sent by this resolver. |
| app.toggle_sidebar_group(group, cx); | ||
| })) | ||
| .on_key_down(cx.listener(move |app, event: &gpui::KeyDownEvent, _, cx| { | ||
| if matches!(event.keystroke.key.as_str(), "enter" | "space") { |
There was a problem hiding this comment.
P3: Modified Enter/Space also toggles this disclosure and stops propagation; ignore modified keystrokes when handling button activation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/native_application/impl_sidebar_threads.rs, line 527:
<comment>Modified Enter/Space also toggles this disclosure and stops propagation; ignore modified keystrokes when handling button activation.</comment>
<file context>
@@ -311,12 +372,186 @@ impl NativeApplication {
+ app.toggle_sidebar_group(group, cx);
+ }))
+ .on_key_down(cx.listener(move |app, event: &gpui::KeyDownEvent, _, cx| {
+ if matches!(event.keystroke.key.as_str(), "enter" | "space") {
+ cx.stop_propagation();
+ app.toggle_sidebar_group(group, cx);
</file context>
| events.append(event) | ||
| events.extend(boundaries.values()) | ||
| events.sort(key=lambda e: e.get('ts', 0)) | ||
| Path(output).write_text(json.dumps({'traceEvents': events, 'displayTimeUnit': 'ms', |
There was a problem hiding this comment.
P3: merge discards every source capture's metadata, so the merged file keeps only sources. Running summary on the merged output then reports unknown capture: unknown and Dropped: 0; evicted: 0 even when the inputs carried real dropped_records/evicted_records counts, and session_start_us/role are gone — the same fields merge uses to dedupe process ids are lost if the merged file is merged again, which reassigns different pids to the same process and breaks the stated deduplication. Carry the union of the inputs' metadata into the output instead of dropping it.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At scripts/trace.py, line 85:
<comment>`merge` discards every source capture's metadata, so the merged file keeps only `sources`. Running `summary` on the merged output then reports `unknown capture: unknown` and `Dropped: 0; evicted: 0` even when the inputs carried real `dropped_records`/`evicted_records` counts, and `session_start_us`/`role` are gone — the same fields `merge` uses to dedupe process ids are lost if the merged file is merged again, which reassigns different pids to the same process and breaks the stated deduplication. Carry the union of the inputs' metadata into the output instead of dropping it.</comment>
<file context>
@@ -0,0 +1,106 @@
+ events.append(event)
+ events.extend(boundaries.values())
+ events.sort(key=lambda e: e.get('ts', 0))
+ Path(output).write_text(json.dumps({'traceEvents': events, 'displayTimeUnit': 'ms',
+ 'metadata': {'sources': [str(p) for p in paths]}}))
+ print(f'Saved {len(events)} events to {output}')
</file context>
| let running = Arc::new(AtomicBool::new(true)); | ||
| let watched = Arc::clone(&stamp); | ||
| let active = Arc::clone(&running); | ||
| let _ = std::thread::Builder::new() |
There was a problem hiding this comment.
P3: If the watchdog thread fails to spawn, monitoring silently disappears while Heartbeat still behaves as if active and tick() keeps updating the stamp. The stall/resume detection this API advertises never runs and nothing reports the failure. Match the recorder: log a message when spawn fails.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/tracing/src/watchdog.rs, line 23:
<comment>If the watchdog thread fails to spawn, monitoring silently disappears while `Heartbeat` still behaves as if active and `tick()` keeps updating the stamp. The stall/resume detection this API advertises never runs and nothing reports the failure. Match the recorder: log a message when spawn fails.</comment>
<file context>
@@ -0,0 +1,80 @@
+ let running = Arc::new(AtomicBool::new(true));
+ let watched = Arc::clone(&stamp);
+ let active = Arc::clone(&running);
+ let _ = std::thread::Builder::new()
+ .name("artisan-trace-watchdog".into())
+ .spawn(move || {
</file context>
| view.switch_engine(engine_id.clone(), cx); | ||
| })) | ||
| }) | ||
| .when(!usable, |tab| tab.opacity(0.35)) |
There was a problem hiding this comment.
P3: A greyed engine's tab is still exposed as role=Button with a plain aria_label, but has no action and no aria_disabled. Screen readers announce an actionable button that does nothing when focused or activated. Set aria_disabled (and keep the "(unavailable)" suffix in the label if desired).
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/native_model_selector/render/menu.rs, line 293:
<comment>A greyed engine's tab is still exposed as `role=Button` with a plain `aria_label`, but has no action and no `aria_disabled`. Screen readers announce an actionable button that does nothing when focused or activated. Set `aria_disabled` (and keep the "(unavailable)" suffix in the label if desired).</comment>
<file context>
@@ -275,17 +275,28 @@ impl NativeModelSelector {
+ view.switch_engine(engine_id.clone(), cx);
+ }))
+ })
+ .when(!usable, |tab| tab.opacity(0.35))
.role(gpui::Role::Button)
- .aria_label(harness.label.clone())
</file context>
| .when(!usable, |tab| tab.opacity(0.35)) | |
| .when(!usable, |tab| tab.opacity(0.35).aria_disabled(true)) |
Two things made counting labels resize horizontally. Spline Sans digits
are proportional ("1" is a third narrower than "8"), so every tick
changed the width; the header, chain clauses, the status line, and the
odometer now use tabular figures (`tnum`, one 1156-unit advance for all
digits). And a live phrase dropped back to plain text between changes,
so the timer swapped two layouts that measure differently every second;
it now keeps its word-by-word layout for as long as the turn is live.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
4 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/assets/src/fonts.rs">
<violation number="1" location="modules/assets/src/fonts.rs:106">
P2: Adding the sixth catalog entry makes the existing integration test fail: `tests/ui/typography_gradient.rs` (bundled_catalog_matches_the_theme_declarations_on_both_sides, unchanged in this PR) asserts `bundled_fonts::ALL.len() == 5` and pins the exact five weight/family ranges, both of which now mismatch (6 entries). Update that test's expected ranges (or the pinned vector literal) in the same change that extends the catalog.</violation>
</file>
<file name="modules/frontend/src/native_application/impl_surfaces.rs">
<violation number="1" location="modules/frontend/src/native_application/impl_surfaces.rs:259">
P2: The production titlebar wordmark now renders a font that the repo's own provenance note restricts to local evaluation. `modules/assets/licenses/founders-grotesk-test.txt` states the OTF was "Included for local font evaluation at the user's request. This provenance note does not grant redistribution or production rights," and `modules/assets/src/fonts.rs` embeds it via `include_bytes!` and registers it unconditionally, so it ships inside every release binary and renders whenever `desktop_brand()` draws "Artisan Editor" — no debug gate. Keep the wordmark on the already-licensed Artisan Neo (SemiBold) until a production-licensed face is available, or gate this family use behind the evaluation/debug builds that the license contemplates.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/work_motion.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/work_motion.rs:385">
P3: `tabular_figures()` allocates a fresh `Vec` and `Arc` on every call, and it is invoked each paint of every phrase element, live status, exit status, and group header (five call sites, repainted every frame while a turn streams). Cache one instance in a `OnceLock` and clone the `Arc` so steady-state painting reuses the allocation.</violation>
<violation number="2" location="modules/frontend/src/conversation_surface/work_motion.rs:392">
P3: While the turn is live, `phrase()` now paints word by word even at rest, and `phrase_element` renders every word as a `flex_none` run inside an `overflow_hidden` row. A label that overflows its column (the elapsed header or a long chain clause in a narrow sidebar) now clips at a word boundary with no ellipsis, where the pre-change at-rest path painted a single text run and `truncate()` ellipsized with "…". This is the deliberate stability trade-off of keeping one layout, but the overflow behavior change is worth confirming against the sidebar widths; if clipping is unacceptable, only overflow handling (not the layout swap) needs revisiting.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| ], | ||
| }, | ||
| BundledFont { | ||
| file_name: "test-founders-grotesk-bold.otf", |
There was a problem hiding this comment.
P2: Adding the sixth catalog entry makes the existing integration test fail: tests/ui/typography_gradient.rs (bundled_catalog_matches_the_theme_declarations_on_both_sides, unchanged in this PR) asserts bundled_fonts::ALL.len() == 5 and pins the exact five weight/family ranges, both of which now mismatch (6 entries). Update that test's expected ranges (or the pinned vector literal) in the same change that extends the catalog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/assets/src/fonts.rs, line 106:
<comment>Adding the sixth catalog entry makes the existing integration test fail: `tests/ui/typography_gradient.rs` (bundled_catalog_matches_the_theme_declarations_on_both_sides, unchanged in this PR) asserts `bundled_fonts::ALL.len() == 5` and pins the exact five weight/family ranges, both of which now mismatch (6 entries). Update that test's expected ranges (or the pinned vector literal) in the same change that extends the catalog.</comment>
<file context>
@@ -102,6 +102,15 @@ pub const ALL: &[BundledFont] = &[
],
},
+ BundledFont {
+ file_name: "test-founders-grotesk-bold.otf",
+ family: "Test Founders Grotesk",
+ weights: (700, 700),
</file context>
| .font_family("Test Founders Grotesk") | ||
| .font_weight(FontWeight::BOLD) | ||
| // -0.025em tracking at 20px matches the Founders specimen. | ||
| .letter_spacing(px(-0.5)) |
There was a problem hiding this comment.
P2: The production titlebar wordmark now renders a font that the repo's own provenance note restricts to local evaluation. modules/assets/licenses/founders-grotesk-test.txt states the OTF was "Included for local font evaluation at the user's request. This provenance note does not grant redistribution or production rights," and modules/assets/src/fonts.rs embeds it via include_bytes! and registers it unconditionally, so it ships inside every release binary and renders whenever desktop_brand() draws "Artisan Editor" — no debug gate. Keep the wordmark on the already-licensed Artisan Neo (SemiBold) until a production-licensed face is available, or gate this family use behind the evaluation/debug builds that the license contemplates.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/native_application/impl_surfaces.rs, line 259:
<comment>The production titlebar wordmark now renders a font that the repo's own provenance note restricts to local evaluation. `modules/assets/licenses/founders-grotesk-test.txt` states the OTF was "Included for local font evaluation at the user's request. This provenance note does not grant redistribution or production rights," and `modules/assets/src/fonts.rs` embeds it via `include_bytes!` and registers it unconditionally, so it ships inside every release binary and renders whenever `desktop_brand()` draws "Artisan Editor" — no debug gate. Keep the wordmark on the already-licensed Artisan Neo (SemiBold) until a production-licensed face is available, or gate this family use behind the evaluation/debug builds that the license contemplates.</comment>
<file context>
@@ -256,10 +256,10 @@ impl NativeApplication {
- .font_weight(FontWeight::SEMIBOLD)
- // -0.05em tracking at 20px: 20 * -0.05 = -1.0px.
- .letter_spacing(px(-1.0))
+ .font_family("Test Founders Grotesk")
+ .font_weight(FontWeight::BOLD)
+ // -0.025em tracking at 20px matches the Founders specimen.
</file context>
| .font_family("Test Founders Grotesk") | |
| .font_weight(FontWeight::BOLD) | |
| // -0.025em tracking at 20px matches the Founders specimen. | |
| .letter_spacing(px(-0.5)) | |
| .font_family("Artisan Neo") | |
| .font_weight(FontWeight::SEMIBOLD) | |
| // -0.05em tracking at 20px: 20 * -0.05 = -1.0px. | |
| .letter_spacing(px(-1.0)) |
| /// counting number never changes width as its digits change. Spline Sans | ||
| /// digits are proportional by default ("1" is a third narrower than "8"). | ||
| pub(super) fn tabular_figures() -> FontFeatures { | ||
| FontFeatures(Arc::new(vec![("tnum".into(), 1)])) |
There was a problem hiding this comment.
P3: tabular_figures() allocates a fresh Vec and Arc on every call, and it is invoked each paint of every phrase element, live status, exit status, and group header (five call sites, repainted every frame while a turn streams). Cache one instance in a OnceLock and clone the Arc so steady-state painting reuses the allocation.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/work_motion.rs, line 385:
<comment>`tabular_figures()` allocates a fresh `Vec` and `Arc` on every call, and it is invoked each paint of every phrase element, live status, exit status, and group header (five call sites, repainted every frame while a turn streams). Cache one instance in a `OnceLock` and clone the `Arc` so steady-state painting reuses the allocation.</comment>
<file context>
@@ -362,9 +378,18 @@ pub(super) fn with_entrance(element: AnyElement, progress: Option<f32>) -> AnyEl
+/// counting number never changes width as its digits change. Spline Sans
+/// digits are proportional by default ("1" is a third narrower than "8").
+pub(super) fn tabular_figures() -> FontFeatures {
+ FontFeatures(Arc::new(vec![("tnum".into(), 1)]))
+}
+
</file context>
| FontFeatures(Arc::new(vec![("tnum".into(), 1)])) | |
| static TABULAR: std::sync::OnceLock<FontFeatures> = std::sync::OnceLock::new(); | |
| TABULAR | |
| .get_or_init(|| FontFeatures(Arc::new(vec![("tnum".into(), 1)]))) | |
| .clone() |
| /// tabular figures. | ||
| pub(super) fn phrase_element(words: &[PhraseWord], line_height: Pixels) -> AnyElement { | ||
| let mut row = div() | ||
| .font_features(tabular_figures()) |
There was a problem hiding this comment.
P3: While the turn is live, phrase() now paints word by word even at rest, and phrase_element renders every word as a flex_none run inside an overflow_hidden row. A label that overflows its column (the elapsed header or a long chain clause in a narrow sidebar) now clips at a word boundary with no ellipsis, where the pre-change at-rest path painted a single text run and truncate() ellipsized with "…". This is the deliberate stability trade-off of keeping one layout, but the overflow behavior change is worth confirming against the sidebar widths; if clipping is unacceptable, only overflow handling (not the layout swap) needs revisiting.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/work_motion.rs, line 392:
<comment>While the turn is live, `phrase()` now paints word by word even at rest, and `phrase_element` renders every word as a `flex_none` run inside an `overflow_hidden` row. A label that overflows its column (the elapsed header or a long chain clause in a narrow sidebar) now clips at a word boundary with no ellipsis, where the pre-change at-rest path painted a single text run and `truncate()` ellipsized with "…". This is the deliberate stability trade-off of keeping one layout, but the overflow behavior change is worth confirming against the sidebar widths; if clipping is unacceptable, only overflow handling (not the layout swap) needs revisiting.</comment>
<file context>
@@ -362,9 +378,18 @@ pub(super) fn with_entrance(element: AnyElement, progress: Option<f32>) -> AnyEl
+/// tabular figures.
pub(super) fn phrase_element(words: &[PhraseWord], line_height: Pixels) -> AnyElement {
let mut row = div()
+ .font_features(tabular_figures())
.flex()
.flex_row()
</file context>
Prose in a turn's work counted as streaming for as long as its message was live, and a message can stay live until its turn ends, so the reveal held back the last word of every finished sentence and the work read cut off. Prose followed by another row, or by the reply, is now finished and reveals to its end. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A revealed segment now splits into short runs whose fades start a frame apart, so it sweeps in left to right and settles instead of spawning; the sweep stays within 160 ms, far inside the fade, so it never reads as letters streaming. Prose that appears while its row is on screen streams in even when it lands whole, as a reply does when the turn ends, and a backlog drains in capped segments instead of appearing at once. A lone `*`, `_`, or `<` followed by a space no longer holds back the rest of its paragraph. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Entering tool chains, chain steps, prose, and the status line no longer blur and rise 12 px over their neighbours while the space below them jumps open. The row's height now opens from nothing over the fast motion token, so the rows below glide down, and its content fades in a beat later while settling 4 px into place. The blur ran the row through a filter layer that softened its text. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 existing issue remains and 3 new issues found across 7 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/frontend/src/conversation_surface/reply_reveal.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/reply_reveal.rs:160">
P2: Adding six bytes can place a fade mark inside a multibyte UTF-8 character. `compile_text_runs` drops ranges with non-character-boundary endpoints, so parts of non-ASCII text skip the fade; place each run boundary at a character boundary.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/work_motion.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/work_motion.rs:35">
P3: The module-level motion description still documents the old 500 ms, 12 px, blurred entrance. Update it to describe the new 400 ms height-opening and 4 px content-rise behavior.</violation>
<violation number="2" location="modules/frontend/src/conversation_surface/work_motion.rs:189">
P2: A prose body that appears after the row's first paint starts a draining reveal even when the turn is not live: `advance()` is reached with `MotionPolicy::Full` for `Pace::Settled`, and `begin(now)` drains the whole body in animated segments. The doc comment on `reveal()` promises "settled history never animates", but a settled turn that appends a new block after the row already painted (e.g. a late-resolving draft or thread detail) will replay its text instead of painting it whole, and `self.animating = true` keeps the settled row repainting for the ~1 s drain. The turn-end drain is wanted, but only when the settle is the finish of the streaming turn; gate `begin` on liveness rather than on `primed` alone.</violation>
</file>
Requires human review: Auto-approval blocked because this review re-detected 1 unresolved issue already reported by Cubic.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| let stagger = SWEEP_STAGGER.min(SWEEP_SPAN / u32::try_from(runs).unwrap_or(u32::MAX)); | ||
| for run in 0..runs { | ||
| let delay = stagger * u32::try_from(run).unwrap_or(u32::MAX); | ||
| self.marks.push((start + run * SWEEP_RUN, began + delay)); |
There was a problem hiding this comment.
P2: Adding six bytes can place a fade mark inside a multibyte UTF-8 character. compile_text_runs drops ranges with non-character-boundary endpoints, so parts of non-ASCII text skip the fade; place each run boundary at a character boundary.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/reply_reveal.rs, line 160:
<comment>Adding six bytes can place a fade mark inside a multibyte UTF-8 character. `compile_text_runs` drops ranges with non-character-boundary endpoints, so parts of non-ASCII text skip the fade; place each run boundary at a character boundary.</comment>
<file context>
@@ -95,35 +115,58 @@ impl ReplyReveal {
+ let stagger = SWEEP_STAGGER.min(SWEEP_SPAN / u32::try_from(runs).unwrap_or(u32::MAX));
+ for run in 0..runs {
+ let delay = stagger * u32::try_from(run).unwrap_or(u32::MAX);
+ self.marks.push((start + run * SWEEP_RUN, began + delay));
+ }
+ }
</file context>
| let now = Instant::now(); | ||
| if self.seen_prose.insert(id.clone()) { | ||
| if self.primed { | ||
| self.reveals.insert(id.clone(), ReplyReveal::begin(now)); |
There was a problem hiding this comment.
P2: A prose body that appears after the row's first paint starts a draining reveal even when the turn is not live: advance() is reached with MotionPolicy::Full for Pace::Settled, and begin(now) drains the whole body in animated segments. The doc comment on reveal() promises "settled history never animates", but a settled turn that appends a new block after the row already painted (e.g. a late-resolving draft or thread detail) will replay its text instead of painting it whole, and self.animating = true keeps the settled row repainting for the ~1 s drain. The turn-end drain is wanted, but only when the settle is the finish of the streaming turn; gate begin on liveness rather than on primed alone.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/work_motion.rs, line 189:
<comment>A prose body that appears after the row's first paint starts a draining reveal even when the turn is not live: `advance()` is reached with `MotionPolicy::Full` for `Pace::Settled`, and `begin(now)` drains the whole body in animated segments. The doc comment on `reveal()` promises "settled history never animates", but a settled turn that appends a new block after the row already painted (e.g. a late-resolving draft or thread detail) will replay its text instead of painting it whole, and `self.animating = true` keeps the settled row repainting for the ~1 s drain. The turn-end drain is wanted, but only when the settle is the finish of the streaming turn; gate `begin` on liveness rather than on `primed` alone.</comment>
<file context>
@@ -170,9 +184,13 @@ impl WorkMotion {
- .insert(id.clone(), ReplyReveal::begin(body, now));
+ if self.seen_prose.insert(id.clone()) {
+ if self.primed {
+ self.reveals.insert(id.clone(), ReplyReveal::begin(now));
+ } else if streaming {
+ self.reveals
</file context>
| use super::*; | ||
|
|
||
| /// Row entrance: `--duration-very-slow` (texts reveal). | ||
| const ENTRANCE: Duration = Duration::from_millis(400); |
There was a problem hiding this comment.
P3: The module-level motion description still documents the old 500 ms, 12 px, blurred entrance. Update it to describe the new 400 ms height-opening and 4 px content-rise behavior.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/work_motion.rs, line 35:
<comment>The module-level motion description still documents the old 500 ms, 12 px, blurred entrance. Update it to describe the new 400 ms height-opening and 4 px content-rise behavior.</comment>
<file context>
@@ -30,11 +32,15 @@ use super::reply_reveal::ReplyReveal;
-const ENTRANCE_RISE_PX: f32 = 12.0;
-/// Row entrance blur: `--blur-medium` (texts reveal).
-const ENTRANCE_BLUR_PX: f32 = 3.0;
+const ENTRANCE: Duration = Duration::from_millis(400);
+/// The row's height opening (`--duration-fast`, accordion/card resize).
+const ENTRANCE_GROW: Duration = MotionDuration::Fast.as_duration();
</file context>
Segments used to take whatever had become stable since the last tick, so the reveal sped up and stalled with the transport's bursts. A word budget now accrues at a rate that eases (0.5 s time constant) toward clearing the backlog within 1.5 s, held between 300 and 900 words per minute, and banks at most six words so a pause never ends in a burst. A fence or table costs at most six words. Replies that land whole drain at the same pace. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 1 file (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/frontend/src/conversation_surface/reply_reveal.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/reply_reveal.rs:166">
P3: After a fence or table unit (cost capped at MAX_UNIT_COST) the budget goes negative, and the `self.budget >= 1.0` gates then withhold every reveal until the debt repays — up to ~1 s at the MIN rate, measured 0.42 s even at the MAX rate. Streamed words sit stable and unrevealed during that pause, so the effective reveal drops to 0 wpm, contradicting the module's 'slowest prose reveals while words wait' floor. Cap the per-spend overhang (e.g. floor the budget at 0) so a costly unit delays at most one segment interval, or document the pause.</violation>
<violation number="2" location="modules/frontend/src/conversation_surface/reply_reveal.rs:186">
P2: The first stable text can inherit idle time since `begin`, filling the budget and revealing up to six words on one frame. Accrue only while stable text is waiting, and reset `paced_at` when there is no backlog.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| /// window, between [`MIN_WORDS_PER_MINUTE`] and [`MAX_WORDS_PER_MINUTE`], | ||
| /// and earns budget at it. | ||
| fn pace(&mut self, backlog: usize, now: Instant) { | ||
| let elapsed = now.saturating_duration_since(self.paced_at).as_secs_f64(); |
There was a problem hiding this comment.
P2: The first stable text can inherit idle time since begin, filling the budget and revealing up to six words on one frame. Accrue only while stable text is waiting, and reset paced_at when there is no backlog.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/reply_reveal.rs, line 186:
<comment>The first stable text can inherit idle time since `begin`, filling the budget and revealing up to six words on one frame. Accrue only while stable text is waiting, and reset `paced_at` when there is no backlog.</comment>
<file context>
@@ -147,6 +179,20 @@ impl ReplyReveal {
+ /// window, between [`MIN_WORDS_PER_MINUTE`] and [`MAX_WORDS_PER_MINUTE`],
+ /// and earns budget at it.
+ fn pace(&mut self, backlog: usize, now: Instant) {
+ let elapsed = now.saturating_duration_since(self.paced_at).as_secs_f64();
+ self.paced_at = now;
+ let backlog = f64::from(u32::try_from(backlog).unwrap_or(u32::MAX));
</file context>
| let mut end = start; | ||
| while end < stable && self.budget >= 1.0 { | ||
| let next = steps.next(end); | ||
| self.budget -= unit_cost(&body[end..next]); |
There was a problem hiding this comment.
P3: After a fence or table unit (cost capped at MAX_UNIT_COST) the budget goes negative, and the self.budget >= 1.0 gates then withhold every reveal until the debt repays — up to ~1 s at the MIN rate, measured 0.42 s even at the MAX rate. Streamed words sit stable and unrevealed during that pause, so the effective reveal drops to 0 wpm, contradicting the module's 'slowest prose reveals while words wait' floor. Cap the per-spend overhang (e.g. floor the budget at 0) so a costly unit delays at most one segment interval, or document the pause.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/reply_reveal.rs, line 166:
<comment>After a fence or table unit (cost capped at MAX_UNIT_COST) the budget goes negative, and the `self.budget >= 1.0` gates then withhold every reveal until the debt repays — up to ~1 s at the MIN rate, measured 0.42 s even at the MAX rate. Streamed words sit stable and unrevealed during that pause, so the effective reveal drops to 0 wpm, contradicting the module's 'slowest prose reveals while words wait' floor. Cap the per-spend overhang (e.g. floor the budget at 0) so a costly unit delays at most one segment interval, or document the pause.</comment>
<file context>
@@ -122,22 +154,22 @@ impl ReplyReveal {
+ let mut end = start;
+ while end < stable && self.budget >= 1.0 {
+ let next = steps.next(end);
+ self.budget -= unit_cost(&body[end..next]);
+ end = next;
}
</file context>
| self.budget -= unit_cost(&body[end..next]); | |
| self.budget = (self.budget - unit_cost(&body[end..next])).max(0.0); |
Every catalog read, and every send (which resolves its model against the catalog), awaited a full probe of the engine CLIs whenever the discovery cache was cold or stale: up to 15 s for OpenCode, and every 10 s while any installed engine's probe failed. Traces showed catalog reads of 7-16 s and a 7.5 s send, with the Editor's other commands queued behind them. Discovery now serves the cached bundle at once and refreshes it in one background probe that never holds the cache lock; only a cold Forge waits, and the Forge starts that first probe at startup. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Commits stamp a running dispatch with max(now, previous stamp), but lease renewal compared that stamp with the raw wall clock and failed as a broken invariant whenever the clock had stepped back. On WSL, where the host-time sync steps the guest clock, every heartbeat of a running turn failed for minutes after the host was locked. Renewal no longer moves the stamp backwards and accepts a stamp later than its own clock reading. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Editor reads the dev Forge's credentials from the WSL home. While WSL
hung, those reads failed and every retry reported an authentication
failure ("the saved connection credential is no longer usable"), so the
window flickered between that screen and connected. An I/O failure
reading credentials now reports the host as unavailable (Forge offline);
invalid credential material still reports authentication.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Three things lost the traces that would have explained today's stall: - a reason repeating every few seconds saved a snapshot each time and rotated earlier, distinct incidents out of the folder; a reason now saves at most once per five minutes; - the Forge wrote to /tmp, which a WSL restart empties; Linux traces now default to $XDG_STATE_HOME/artisan/traces (else ~/.local/state); - navigation gates were recorded on every service event, crowding the bounded history; they are recorded only when they change. A refusal the Forge answered (a link title it could not fetch) is still recorded but no longer counts as an incident. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each stage's dependency cache ran crane's `cargo check` before `cargo build`; nothing reads those artifacts, and it cost about 2.5 minutes of the 13-minute rebuild after a lockfile or gpui change. Payload and runner builds also copied about 1 GB of binaries that `installPhaseCommand` already moves (about 30 s per payload). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
10 issues found across 38 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/frontend/src/native_application/tests/new_thread_send.rs">
<violation number="1" location="modules/frontend/src/native_application/tests/new_thread_send.rs:260">
P2: This one-line match is not rustfmt-formatted, so the required `cargo fmt --all -- --check` CI step fails. Run rustfmt on this file.</violation>
</file>
<file name="modules/frontend/src/native_transport_service/service_lifecycle.rs">
<violation number="1" location="modules/frontend/src/native_transport_service/service_lifecycle.rs:459">
P2: This treats every credential I/O failure as a Forge outage, including local permission or filesystem errors when the Forge is running. Preserve the I/O cause and use `Unreachable` only for errors that indicate the host is unavailable.</violation>
</file>
<file name="modules/frontend/src/native_transport_service/request_delivery.rs">
<violation number="1" location="modules/frontend/src/native_transport_service/request_delivery.rs:300">
P2: `Retained` also covers request-ID mismatches and response-family integrity failures, not just peer refusals; these now log `request.failed` without saving an incident snapshot. Trigger `incident!` for non-`Peer` retained failures while keeping peer refusals excluded.</violation>
</file>
<file name="modules/tracing/src/recorder.rs">
<violation number="1" location="modules/tracing/src/recorder.rs:421">
P2: `send_snapshot` can drop a snapshot when its writer queue is full, but this records the reason as handled anyway; repeat incidents are then suppressed for five minutes without a trace. Update `last_by_reason` only when the snapshot is successfully enqueued.</violation>
</file>
<file name="modules/backend/src/request_handler/user_preferences.rs">
<violation number="1" location="modules/backend/src/request_handler/user_preferences.rs:149">
P2: Agent and system saves are no-ops, but this arm still wakes every connection and makes each reread preferences. Wake only for user-sourced saves to avoid that fan-out for runtime choices.</violation>
</file>
<file name="modules/database/src/repository/message_dispatch/lease.rs">
<violation number="1" location="modules/database/src/repository/message_dispatch/lease.rs:28">
P2: When another Forge shares the database, a backward clock step can make this accepted renewal store an expiry earlier than the persisted one. That peer's recovery sweep can then dispose the live run after the clock catches up; preserve the expiry high-water mark too.</violation>
</file>
<file name="modules/frontend/src/native_application/flight_recorder.rs">
<violation number="1" location="modules/frontend/src/native_application/flight_recorder.rs:107">
P2: Unchanged gates are suppressed indefinitely, but the recorder expires events after 120 seconds and can drop them on a full queue, leaving later incident snapshots without the current navigation state. Re-emit a low-frequency checkpoint instead of deduplicating forever.</violation>
</file>
<file name="modules/domain/src/user_preferences.rs">
<violation number="1" location="modules/domain/src/user_preferences.rs:10">
P3: This rustdoc says “a saved explicit user model choices,” which is ungrammatical and obscures the rule. Use singular “explicit user model choice.”</violation>
</file>
<file name="modules/backend/src/request_handler/project_draft_submission.rs">
<violation number="1" location="modules/backend/src/request_handler/project_draft_submission.rs:40">
P3: The module docs still say a selected send makes that configuration the default, but this path now leaves preferences untouched. Update the module description to match the separate explicit preference-saving flow.</violation>
</file>
<file name="modules/backend/src/request_handler.rs">
<violation number="1" location="modules/backend/src/request_handler.rs:1085">
P2: Manual Engine Settings saves still submit `SetThreadEngineConfig`, but this branch no longer updates the Forge default, so new threads lose the saved model choice. Persist an explicit user preference for this save path as well.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| let later = catalog.selection_policy_for_model("codex-sol").unwrap(); | ||
| application.handle_composer_policy_selection(&chosen, cx); | ||
| let saved = commands.borrow().iter().find_map(|command| match command { | ||
| NativeTransportCommand::Preferences(crate::native_transport_service::PreferencesCommand::SaveModel(save)) => Some(save.clone()), |
There was a problem hiding this comment.
P2: This one-line match is not rustfmt-formatted, so the required cargo fmt --all -- --check CI step fails. Run rustfmt on this file.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/native_application/tests/new_thread_send.rs, line 260:
<comment>This one-line match is not rustfmt-formatted, so the required `cargo fmt --all -- --check` CI step fails. Run rustfmt on this file.</comment>
<file context>
@@ -256,6 +256,12 @@ fn a_model_chosen_in_the_new_task_draft_stays_with_it_and_is_what_send_carries(
let later = catalog.selection_policy_for_model("codex-sol").unwrap();
application.handle_composer_policy_selection(&chosen, cx);
+ let saved = commands.borrow().iter().find_map(|command| match command {
+ NativeTransportCommand::Preferences(crate::native_transport_service::PreferencesCommand::SaveModel(save)) => Some(save.clone()),
+ _ => None,
+ }).expect("a picker choice saves preferences before a new task is sent");
</file context>
| // Forge as offline instead of asking for a fresh invitation. | ||
| if matches!( | ||
| error, | ||
| artisan_editor_cli::credentials::ForgeCredentialError::Io { .. } |
There was a problem hiding this comment.
P2: This treats every credential I/O failure as a Forge outage, including local permission or filesystem errors when the Forge is running. Preserve the I/O cause and use Unreachable only for errors that indicate the host is unavailable.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/native_transport_service/service_lifecycle.rs, line 459:
<comment>This treats every credential I/O failure as a Forge outage, including local permission or filesystem errors when the Forge is running. Preserve the I/O cause and use `Unreachable` only for errors that indicate the host is unavailable.</comment>
<file context>
@@ -446,11 +446,22 @@ async fn start_dev_service(home: &Path) -> Result<(ServiceRuntime, FrameFactory)
+ // Forge as offline instead of asking for a fresh invitation.
+ if matches!(
+ error,
+ artisan_editor_cli::credentials::ForgeCredentialError::Io { .. }
+ ) {
+ StartupError::Unreachable(ServiceFailureStage::Credentials)
</file context>
| Err(RequestAttemptError::Retained { failure, .. }) => { | ||
| artisan_tracing::instant!("transport.request", "request.failed", "stage" => failure.stage.to_string(), "category" => failure.category.to_string()); | ||
| } |
There was a problem hiding this comment.
P2: Retained also covers request-ID mismatches and response-family integrity failures, not just peer refusals; these now log request.failed without saving an incident snapshot. Trigger incident! for non-Peer retained failures while keeping peer refusals excluded.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/native_transport_service/request_delivery.rs, line 300:
<comment>`Retained` also covers request-ID mismatches and response-family integrity failures, not just peer refusals; these now log `request.failed` without saving an incident snapshot. Trigger `incident!` for non-`Peer` retained failures while keeping peer refusals excluded.</comment>
<file context>
@@ -294,10 +294,14 @@ impl ServiceRuntime {
+ // The Forge answered with a refusal (a link title it could not
+ // fetch, a stale catalog): the connection is fine, so the
+ // failure is recorded but saves no incident snapshot.
+ Err(RequestAttemptError::Retained { failure, .. }) => {
+ artisan_tracing::instant!("transport.request", "request.failed", "stage" => failure.stage.to_string(), "category" => failure.category.to_string());
+ }
</file context>
| Err(RequestAttemptError::Retained { failure, .. }) => { | |
| artisan_tracing::instant!("transport.request", "request.failed", "stage" => failure.stage.to_string(), "category" => failure.category.to_string()); | |
| } | |
| Err(RequestAttemptError::Retained { failure, .. }) => { | |
| artisan_tracing::instant!("transport.request", "request.failed", "stage" => failure.stage.to_string(), "category" => failure.category.to_string()); | |
| if failure.category != ServiceFailureCategory::Peer { | |
| artisan_tracing::incident!("request.failed"); | |
| } | |
| } |
| pending = None; | ||
| last_incident = Some(now); | ||
| last_by_reason.retain(|(known, _)| *known != reason); | ||
| last_by_reason.push((reason, now)); |
There was a problem hiding this comment.
P2: send_snapshot can drop a snapshot when its writer queue is full, but this records the reason as handled anyway; repeat incidents are then suppressed for five minutes without a trace. Update last_by_reason only when the snapshot is successfully enqueued.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/tracing/src/recorder.rs, line 421:
<comment>`send_snapshot` can drop a snapshot when its writer queue is full, but this records the reason as handled anyway; repeat incidents are then suppressed for five minutes without a trace. Update `last_by_reason` only when the snapshot is successfully enqueued.</comment>
<file context>
@@ -379,18 +403,22 @@ fn collect(
pending = None;
last_incident = Some(now);
+ last_by_reason.retain(|(known, _)| *known != reason);
+ last_by_reason.push((reason, now));
}
}
</file context>
| | Command::ImportLegacyPreferences(_) | ||
| | Command::SetThreadEngineConfig(_) | ||
| | Command::SubmitComposerDraft(_) | ||
| | Command::SaveModelPreference(_) |
There was a problem hiding this comment.
P2: Agent and system saves are no-ops, but this arm still wakes every connection and makes each reread preferences. Wake only for user-sourced saves to avoid that fan-out for runtime choices.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/backend/src/request_handler/user_preferences.rs, line 149:
<comment>Agent and system saves are no-ops, but this arm still wakes every connection and makes each reread preferences. Wake only for user-sourced saves to avoid that fan-out for runtime choices.</comment>
<file context>
@@ -99,27 +99,57 @@ impl RequestHandler {
| Command::ImportLegacyPreferences(_)
- | Command::SetThreadEngineConfig(_)
- | Command::SubmitComposerDraft(_)
+ | Command::SaveModelPreference(_)
) && let Some(notifier) = &self.conversation_commit_notifier
{
</file context>
| UPDATE message_dispatches | ||
| SET lease_expires_at_ms = ?, | ||
| updated_at_ms = CASE WHEN state = 'leased' THEN ? ELSE updated_at_ms END | ||
| updated_at_ms = CASE WHEN state = 'leased' THEN MAX(updated_at_ms, ?) ELSE updated_at_ms END |
There was a problem hiding this comment.
P2: When another Forge shares the database, a backward clock step can make this accepted renewal store an expiry earlier than the persisted one. That peer's recovery sweep can then dispose the live run after the clock catches up; preserve the expiry high-water mark too.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/database/src/repository/message_dispatch/lease.rs, line 28:
<comment>When another Forge shares the database, a backward clock step can make this accepted renewal store an expiry earlier than the persisted one. That peer's recovery sweep can then dispose the live run after the clock catches up; preserve the expiry high-water mark too.</comment>
<file context>
@@ -25,7 +25,7 @@ use super::{DispatchLeaseOwner, TransitionedMessageDispatch};
UPDATE message_dispatches
SET lease_expires_at_ms = ?,
- updated_at_ms = CASE WHEN state = 'leased' THEN ? ELSE updated_at_ms END
+ updated_at_ms = CASE WHEN state = 'leased' THEN MAX(updated_at_ms, ?) ELSE updated_at_ms END
WHERE message_id = ?
AND state IN ('leased', 'running')
</file context>
| }); | ||
| let changed = LAST_GATES.with(|last| { | ||
| let mut last = last.borrow_mut(); | ||
| if last.as_ref() == Some(&gates) { |
There was a problem hiding this comment.
P2: Unchanged gates are suppressed indefinitely, but the recorder expires events after 120 seconds and can drop them on a full queue, leaving later incident snapshots without the current navigation state. Re-emit a low-frequency checkpoint instead of deduplicating forever.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/native_application/flight_recorder.rs, line 107:
<comment>Unchanged gates are suppressed indefinitely, but the recorder expires events after 120 seconds and can drop them on a full queue, leaving later incident snapshots without the current navigation state. Re-emit a low-frequency checkpoint instead of deduplicating forever.</comment>
<file context>
@@ -75,20 +81,38 @@ impl NativeApplication {
+ });
+ let changed = LAST_GATES.with(|last| {
+ let mut last = last.borrow_mut();
+ if last.as_ref() == Some(&gates) {
+ false
+ } else {
</file context>
| self.remember_default_engine_config(config.config()).await; | ||
| } | ||
| response | ||
| self.set_thread_engine_config_outcome(request_id, config.as_ref()) |
There was a problem hiding this comment.
P2: Manual Engine Settings saves still submit SetThreadEngineConfig, but this branch no longer updates the Forge default, so new threads lose the saved model choice. Persist an explicit user preference for this save path as well.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/backend/src/request_handler.rs, line 1085:
<comment>Manual Engine Settings saves still submit `SetThreadEngineConfig`, but this branch no longer updates the Forge default, so new threads lose the saved model choice. Persist an explicit user preference for this save path as well.</comment>
<file context>
@@ -1082,14 +1082,8 @@ impl RequestHandler {
- self.remember_default_engine_config(config.config()).await;
- }
- response
+ self.set_thread_engine_config_outcome(request_id, config.as_ref())
+ .await
}
</file context>
| //! renders them and reports navigation; the Forge owns every rule (the | ||
| //! most-recently-used order, which thread is remembered, when a saved | ||
| //! configuration becomes the default). | ||
| //! explicit user model choices become the default). |
There was a problem hiding this comment.
P3: This rustdoc says “a saved explicit user model choices,” which is ungrammatical and obscures the rule. Use singular “explicit user model choice.”
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/domain/src/user_preferences.rs, line 10:
<comment>This rustdoc says “a saved explicit user model choices,” which is ungrammatical and obscures the rule. Use singular “explicit user model choice.”</comment>
<file context>
@@ -7,7 +7,7 @@
//! renders them and reports navigation; the Forge owns every rule (the
//! most-recently-used order, which thread is remembered, when a saved
-//! configuration becomes the default).
+//! explicit user model choices become the default).
use thiserror::Error;
</file context>
| //! explicit user model choices become the default). | |
| //! explicit user model choice becomes the default). |
| config: EngineRunConfig, | ||
| chosen: bool, | ||
| }, | ||
| /// Create the thread with `config` without changing user preferences. |
There was a problem hiding this comment.
P3: The module docs still say a selected send makes that configuration the default, but this path now leaves preferences untouched. Update the module description to match the separate explicit preference-saving flow.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/backend/src/request_handler/project_draft_submission.rs, line 40:
<comment>The module docs still say a selected send makes that configuration the default, but this path now leaves preferences untouched. Update the module description to match the separate explicit preference-saving flow.</comment>
<file context>
@@ -37,12 +37,8 @@ const NEW_TASK_TITLE: &str = "New task";
- config: EngineRunConfig,
- chosen: bool,
- },
+ /// Create the thread with `config` without changing user preferences.
+ Admitted { config: EngineRunConfig },
/// Refuse the send; nothing is created.
</file context>
A counting number rested as one text run and rolled as one column per character. The font's tracking spaces characters within a run but not across columns, so each tick widened the label by a few pixels for the roll and snapped it back. Numbers now rest in the same columns. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A sidebar row hidden in a collapsed group, or moved into one, still has its thread in the listing, so the pill stayed where the row used to be. The sidebar now clears its hover against the rows this render builds; the turn navigator and the home project menu do the same against the rows they list, before the pill reads its target. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Forge marks a thread read the moment it opens, and the row hid its dot while open, so opening a finished thread pulled it out of the unread rows under the pointer. The sidebar now holds the outcome the open thread had, or gained while open, until the reader leaves it; a new run there acts on it and releases it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude's narration before a tool call counts as the reply while it streams, then moves back into the work when the call starts. Folding on its first word folded the whole section and its working label away, streamed a few words, and reopened it, and the moved prose entered again from zero height. The work now folds once the reply is final, has stopped streaming, or has shown 40 words, and prose already shown as the reply keeps its place when it moves into the work. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Each segment swept its 6-character runs 16 ms apart and then waited for the next segment, so the fade arrived in small bursts and read as stiff. Runs are now 3 characters, spread across the whole segment interval, and each segment's sweep picks up from the previous one's front, so one soft front glides through the text. The fade is 400 ms ease-out, so a gentle gradient trails the front. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A minimized or hidden window stops painting while scenes keep arriving, so its next paint treated everything that landed meanwhile as new: four narration paragraphs streamed in at once on refocus. The surface now records when its oldest unpainted scene arrived; a paint more than 500 ms later is a catch-up, in which rows end their running reveals and paint at rest like a first render. Prose still arriving shows what landed and streams only the rest. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude's reasoning streams, so the status line's thinking summary rewrote itself as each trace arrived. The scene now takes a trace for the summary only once it is whole: no longer live, or followed by any later item. A new summary replacing the last one swaps in with the text swap recipe, fading in as it rises 4 px; the counting verb never swaps. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A live turn's status is derived afresh on every delivery, and only a streaming non-commentary reply, tool activity, or reasoning counted as the provider responding. Codex's commentary preamble, settled prose, and approval or question cards did not, so a turn that had replied and run a tool could fall back to "Waiting for Codex to respond…". Any prose or provider-produced fact now reads as thinking; only a turn with no response at all waits. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the Context section closed with the full-width rule and crosshair, so the Agents, Checklist and Terminals sections ended open below. Each shown section now closes with the same rule, the column inset below its last row; a hidden section takes its rule with it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 12 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/frontend/src/conversation_surface/reply_reveal.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/reply_reveal.rs:214">
P2: A short segment can remain transparent for nearly 150 ms after rendering: this adds the new segment's stagger even when the previous long sweep's final mark is already near `began`. Continue from the previous mark or its actual cadence instead of adding the new segment's stagger.</violation>
</file>
<file name="modules/frontend/src/conversation_surface/transcript_window.rs">
<violation number="1" location="modules/frontend/src/conversation_surface/transcript_window.rs:680">
P2: `caught_up` is consumed by the first viewport build, but off-window turns are placeholders and never receive it. A retained live row updated off-screen then animates its accumulated work when scrolled back; track catch-up state per turn until that row syncs.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| let front = self | ||
| .marks | ||
| .last() | ||
| .map_or(began, |(_, last)| began.max(*last + stagger)); |
There was a problem hiding this comment.
P2: A short segment can remain transparent for nearly 150 ms after rendering: this adds the new segment's stagger even when the previous long sweep's final mark is already near began. Continue from the previous mark or its actual cadence instead of adding the new segment's stagger.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/reply_reveal.rs, line 214:
<comment>A short segment can remain transparent for nearly 150 ms after rendering: this adds the new segment's stagger even when the previous long sweep's final mark is already near `began`. Continue from the previous mark or its actual cadence instead of adding the new segment's stagger.</comment>
<file context>
@@ -200,10 +204,17 @@ impl ReplyReveal {
+ let front = self
+ .marks
+ .last()
+ .map_or(began, |(_, last)| began.max(*last + stagger));
for run in 0..runs {
let delay = stagger * u32::try_from(run).unwrap_or(u32::MAX);
</file context>
| .map_or(began, |(_, last)| began.max(*last + stagger)); | |
| .map_or(began, |(_, last)| began.max(*last)); |
| // a longer wait means nobody watched it arrive. | ||
| let caught_up = self | ||
| .unpainted_since | ||
| .take() |
There was a problem hiding this comment.
P2: caught_up is consumed by the first viewport build, but off-window turns are placeholders and never receive it. A retained live row updated off-screen then animates its accumulated work when scrolled back; track catch-up state per turn until that row syncs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/conversation_surface/transcript_window.rs, line 680:
<comment>`caught_up` is consumed by the first viewport build, but off-window turns are placeholders and never receive it. A retained live row updated off-screen then animates its accumulated work when scrolled back; track catch-up state per turn until that row syncs.</comment>
<file context>
@@ -668,6 +673,12 @@ impl ConversationSurface {
+ // a longer wait means nobody watched it arrive.
+ let caught_up = self
+ .unpainted_since
+ .take()
+ .is_some_and(|since| since.elapsed() >= UNWATCHED_AFTER);
for index in 0..turn_count {
</file context>
Claude reports how much context a run holds but never the window it fills, so its usage carried no denominator and the context meter could not paint. Usage results the Forge pushes or answers now take a missing window from the cached model catalog (Claude's discovery lists max_input_tokens). The lookup never probes, so a usage read never waits on an engine, and a reported window is never replaced. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Editor opened a usage scope only for OpenCode2 runs, so Claude, Codex and Grok reports were dropped as unscoped and the context meter never showed for them. A native engine's run now opens a scope that accepts its reports under the engine's own model and route ids, the thread and run being the attribution. The reading also outlives its run, so an idle thread keeps showing the last context reading instead of hiding the meter the moment the run settles. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
2 issues found across 8 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/frontend/src/composer_queue_state/tests.rs">
<violation number="1" location="modules/frontend/src/composer_queue_state/tests.rs:377">
P2: This only checks that a snapshot exists, so the test still passes if native usage loses or rewrites Claude’s model or route IDs. Assert `model_id` and `engine_id` on the returned usage.</violation>
</file>
<file name="modules/backend/src/model_discovery/mod.rs">
<violation number="1" location="modules/backend/src/model_discovery/mod.rs:168">
P2: OpenCode can list the same native model ID under different providers, each with its own context limit, but this lookup returns whichever provider's row appears first. Include provider identity in the usage lookup or otherwise disambiguate the matching catalog row so the meter receives the selected model's window.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| state.accept_pushed_usage(pushed, "claude-opus-5-5".to_owned()), | ||
| Ok(UsageResultDisposition::Updated) | ||
| ); | ||
| assert!(state.reporting_usage_for(Some(run_id.as_str())).is_some()); |
There was a problem hiding this comment.
P2: This only checks that a snapshot exists, so the test still passes if native usage loses or rewrites Claude’s model or route IDs. Assert model_id and engine_id on the returned usage.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/frontend/src/composer_queue_state/tests.rs, line 377:
<comment>This only checks that a snapshot exists, so the test still passes if native usage loses or rewrites Claude’s model or route IDs. Assert `model_id` and `engine_id` on the returned usage.</comment>
<file context>
@@ -356,6 +356,37 @@ fn stale_usage_is_rejected_without_reinterpreting_the_reporting_model() {
+ state.accept_pushed_usage(pushed, "claude-opus-5-5".to_owned()),
+ Ok(UsageResultDisposition::Updated)
+ );
+ assert!(state.reporting_usage_for(Some(run_id.as_str())).is_some());
+ // The run settled: the idle thread keeps its last reading.
+ assert_eq!(
</file context>
| assert!(state.reporting_usage_for(Some(run_id.as_str())).is_some()); | |
| let usage = state | |
| .reporting_usage_for(Some(run_id.as_str())) | |
| .expect("native usage is available"); | |
| assert_eq!(usage.model_id, "claude-opus-5-5"); | |
| assert_eq!(usage.engine_id, "claude"); |
| bundle | ||
| .models | ||
| .iter() | ||
| .find(|model| model.engine_id == engine_id && model.native_model_id == model_id) |
There was a problem hiding this comment.
P2: OpenCode can list the same native model ID under different providers, each with its own context limit, but this lookup returns whichever provider's row appears first. Include provider identity in the usage lookup or otherwise disambiguate the matching catalog row so the meter receives the selected model's window.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. When an issue isn't valid or won't be fixed in this PR, reply in its thread with the reason and then resolve the thread. At modules/backend/src/model_discovery/mod.rs, line 168:
<comment>OpenCode can list the same native model ID under different providers, each with its own context limit, but this lookup returns whichever provider's row appears first. Include provider identity in the usage lookup or otherwise disambiguate the matching catalog row so the meter receives the selected model's window.</comment>
<file context>
@@ -157,6 +157,18 @@ fn cache() -> &'static Cache {
+ bundle
+ .models
+ .iter()
+ .find(|model| model.engine_id == engine_id && model.native_model_id == model_id)
+ .and_then(|model| model.context_window_tokens)
+}
</file context>
Claude's result frame names each model's contextWindow under modelUsage, but the usage report always carried no window, so the context meter fell back to the catalog at best. The result's report now carries the largest window it names (the root model's; subagents run on smaller ones), and keeps the last response's context gauge, which the result's totals lack, so the final reading stays whole. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The section kept the settled run's subagents indefinitely, so finished agents stayed listed with their completed dots after the thread went idle. Agents now list only while the thread's run is live; a settled run lists none, and a new run starts over. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Summary by cubic
Adds a local flight recorder for debugging stalled UI, transport, and Forge operations, and reworks the live conversation view: shift-free streaming, an inbox sidebar with project icons, approvals that settle as soon as their receipt lands, context usage for every engine, and a catalog that no longer stalls sends on stale CLI probes.
Conversation
Available models:listing is parsed.Debugging
artisan-tracingkeeps bounded traces; a stall, failure, or slow operation saves a Chrome Trace Event JSON file, and Ctrl+Shift+F11 saves manually.%TEMP%\artisan-traceson Windows and$XDG_STATE_HOME/artisan/traceson Linux so WSL restarts no longer wipe them;scripts/trace.pymerges Editor and Forge traces for Perfetto.ARTISAN_TRACE=0; the newgpui-cepin adds paint-time transforms that the work motion relies on.Written for commit 3fdf308. Summary will update on new commits.