Simplification pass over the for_each branch. No behavior change.
Share what was duplicated:
- Node::prompt_or_label replaces the "prompt, else label" fallback that
agent, prompt, and the for_each item injector each wrote out.
- context::lookup_flat replaces the "exact key, then strip context."
lookup that condition.rs had twice and the for_each source had again.
- is_llm_handler_type replaces the inline agent/prompt match, so the
runtime and the for_each_contract rule agree by construction.
- find_join_node now takes branch ids, so a for_each fan-out passes its
template target instead of needing find_join_for_target.
- collect_events moves to test_support; parallel and integration tests
shared one copy already.
- One ScriptedHandler replaces four test handlers that differed only in
what they returned.
Straighten the branch retry loop:
- Reserve the branch scope once before the loop instead of guarding it
with an Option, which removes three expect() calls.
- acquire_branch_permit and backoff_or_cancel replace the cancel-aware
select! blocks the loop repeated verbatim.
- Keep the match arms in Executor::execute_with_retry order so the two
loops stay easy to compare.
Drop redundant state:
- BranchPlan::is_for_each derives from template_target_id.
- for_each_contract checks node type before source shape, so one
mistake reports one diagnostic.
Prove the new wire fields survive the OpenAPI boundary: the fabro-api
round-trip fixture now carries index and item_label.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Conflicts were between this branch's parallel-branch identity work and
main's stage billing, review targets, and live stage timing.
- Stage fixtures: main added `billing` to each per-file `makeStage`; this
branch had hoisted one builder into `lib/test-utils`. Kept the hoisted
builder and gave it `billing: makeBilledTokenCounts()`, so both intents
hold and the field list stays in one place. `stage-sidebar.test.ts` also
builds raw `RunStage` wire payloads, so it keeps importing
`makeBilledTokenCounts` directly.
- Import lists (`run_projection.rs`, `fabro-api/src/lib.rs`,
`run_state.rs`, `stage_projection_round_trip.rs`): unioned both sides —
`ParallelBranchId` alongside `timing`, `ReviewTarget`,
`ReviewTargetKind`, `AttrValue`, `Node`, and
`StageToolBatchProjection`.
- `fabro-server` tests: git interleaved two unrelated new tests into one
body. Split them back into
`list_run_stages_exposes_parallel_branch_identity` and
`run_billing_includes_live_stage_timing_in_rows_and_totals`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Branch indexes are sparse. A branch queued behind max_parallel reserves
no stage identity until it acquires the semaphore, tokio task order is
not index order, and a branch cancelled while queued never reserves one
at all. Sizing the row list by `stagesByBranchIndex.size` treated an
entry count as a dense index range, so a running branch at index 2 with
nothing at 0 or 1 rendered as a single "pending" placeholder and the
running branch disappeared. Size from the highest index observed.
Also:
- Derive the Succeeded/Failed tiles from the rendered rows instead of the
completed-event rollup, so the tiles cannot contradict the list. This
drops the isComplete fork and both live counters.
- Show the fallback branch count in the Branches tile, which previously
read "-" above N rows in exactly the case the fallback exists for.
- BranchRow carries `label` and `stageId`; ChildRow owns the route it
links to. `id` had become a display label on one path and a raw node id
on the other, and a view model should not hold a URL.
- Drop `branchIndex`, which only ever served as the React key and always
equalled the array index.
- Cover the sparse-index and pending-placeholder paths, and derive the
rollup counts in `completedEvent` instead of passing contradictory ones.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Branches bypass the engine's stage.started/stage.completed lifecycle, so
no SWR key invalidated the stages list while a fork ran. The new live
branch rows stayed frozen at their first observed state until an
incidental refetch. Map parallel.* events to the stages list, run events,
and graph keys.
Also:
- Label branch rows with formatStageLabel so a re-entered branch renders
as `review_glm@2`, matching the sidebar and waterfall.
- Build branch rows in one pass and count live outcomes in one loop.
- Name ParallelBranchId in the OpenAPI spec and reuse fabro_types::
ParallelBranchId, replacing two copies of an inline string format.
- Hoist makeStage and textContent into lib/test-utils so widening Stage
cannot leave per-file fixtures stale (tests are excluded from
typecheck, so the two component-test copies had already gone stale).
- Query stat tiles by data-stat instead of an exact Tailwind class.
- Reuse append_scoped_stage_event's body via append_event_with_scope and
add test_branch_event instead of poking envelope fields.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Conflict in apps/fabro-web/app/components/interview-dock.tsx. Main moved
the dock onto the shared collapsible `RunDockShell` and replaced the
local button constants with shared ones.
Kept main's structure whole and re-applied the review target rendering
onto it: the question paragraph in the shell's `body` becomes the linked
`ReviewTargetQuestion` when the target passes `safeReviewTarget`, and
plain text otherwise. Both now share main's paragraph classes through
`QUESTION_TEXT`, so the two renderings stay visually identical.
`peek` keeps using `question.text`, which is the correct plain-text
collapsed summary for a review target question.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The review question sentence was written in four places and the URL
safety rules in three. Collapse each to one definition.
- Add `ReviewTarget::question_text_with_link` as the single definition of
the question wording. `question_text()` and the Slack header both use
it, so a wording change is now one edit.
- Delete `ReviewTargetKind::noun()`. The enum already derives
`strum::Display` with the same snake_case output.
- Share one `review_target_line` helper between the console interviewer
and the CLI attach client, which held a byte-identical copy. Print only
the URL: `question.text` already carries the label and the noun.
- Trim the web-side check to the URL scheme, host, and credentials, which
are what a raw `href` can act on. Label length and control characters
cannot affect the DOM and stay server-side.
- Split validation from presentation in the web UI. `safeReviewTarget`
returns the target or null, and each caller picks its own fallback, so
an unsafe target now falls back to the same Markdown rendering as a
question with no target.
- Derive the resource noun from `kind` in the web UI instead of
hardcoding "document".
- Use `ReviewTargetKind.DOCUMENT` and the shared `isRecord` guard when
parsing events, instead of a raw string and a hand-rolled object check
that accepted arrays.
- Drop `deny_unknown_fields` from the wire struct. The OpenAPI schema
leaves `additionalProperties` permissive, so an added field would
otherwise make persisted events unreadable.
- Import `ReviewTarget` by name, and stop naming Slack in a fabro-types
error message.
- Document that `review_target=true` replaces the gate's `label`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The interview question panel could take half the viewport, and the page
reserved a fixed 18rem beneath it regardless of how tall it actually was,
so a long question covered the stage rows it was asking about.
Add a shared `RunDockShell` for the two controls docked at the bottom of
the run detail route. It is three zones: a header that is always visible
and doubles as the collapsed bar, a body that scrolls, and actions that
stay pinned so the controls needed to answer or send never scroll out of
reach.
The interview dock drops the question-type subtitle the answer buttons
already state, turns the 160px context box into a closed disclosure with
a first-line preview, drops the "or" divider row, and reveals the
keyboard hint on focus inside the composer row instead of standing below
it. Options stack into a list once a label is too long to sit in a pill.
For the sample question this is 506px down to 325px, or 43px collapsed.
The steering dock gains the same header. `Interrupt` moves into it,
because it acts on the run rather than on the message being composed, and
the waiting notice folds into the header status instead of adding a row.
Both docks now share one composer.
Clearance is measured from the rendered dock rather than assumed. The
former constants remain as the pre-measurement first frame.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The artifacts page grouped captures by stage, which is the storage key
`(stage, retry, path)` rather than anything a reader thinks in. A file
rewritten by four stages appeared as four separate rows under four
headings, with no indication they were the same file.
Group by path instead. Each file is one row showing its latest capture;
earlier captures disclose inline behind a chevron with the producing
stage, size, and the byte change that capture introduced.
Three fixes fall out of the regrouping:
- Order versions by the producing stage's `startedAt`. The previous sort
was alphabetical by stage label, which scrambled history — a report
that grew 8.42 KB -> 13.16 -> 14.32 -> 17.48 rendered newest-first
under a heading implying it was the earliest.
- Drop captures from graph control nodes (`start`, `exit`) via the
existing `isVisibleStage` helper. Those nodes run no work, so the
files they match are pre-existing workspace files swept up by the
capture globs, not run output. This is display-side only; the capture
path still stores them.
- Show the retry badge at `retry > 1` rather than `retry > 0`. Attempts
are 1-based, so the old condition matched every capture and rendered
a "retry 1" badge on every group.
Grouping lives in a separate module so it is testable without React.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The model indicator on a stage page hovered to provider, model, and
reasoning effort only. Seeing what a stage actually spent meant leaving
for the Billing tab, which reports per node rather than per visit.
The stage list had no token data to show, so add a per-visit `billing`
block to `GET /runs/{id}/stages`. The Billing tab's pricing rule (a
provider-reported cost wins, otherwise the server catalog prices the
tokens) was private to `billing_rollup`; move it to
`StageProjection::billed_usage` and drive both call sites from it so the
two views cannot drift.
The popover's buckets use the Billing tab's labels verbatim. It stays
scoped to one visit, so a looped node's row on the Billing tab is the sum
of what each of its visits shows here.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The board cards showed wall-clock duration in the footer's bottom-right
corner. Replace it with the same SizeChip the list view and run detail
header use, so the cost signal is consistent across all three views.
The chip inherits the tooltip, which names the tier and adds the cost
once a run has terminal billing.
Add SizeChip tests pinning the tooltip label for each tier.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The Size column in the runs list rendered SizeChip without the billed
total, so its tooltip read "Size M" while the run detail header showed
"Size M · $12.34 billed".
The tooltip was also unreachable: the row title link paints a
`before:absolute before:inset-0` overlay across the whole row, which sat
above the chip and swallowed hover. Wrapping the chip in `relative z-10`
lifts it above that overlay, matching how the created-by and pull request
cells already handle interactive content.
Runs without terminal billing keep the plain "Size M" label, same as the
header.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`active_time_ms` was only ever computed from terminal stage events, so a
stage still running contributed zero to the run rollup. A run parked in one
long agent stage reported 2m 8s of active time against 16m 53s of wall
clock — the two finished stages — while the running stage had been doing
continuous inference and tool work for over 14 minutes.
`live_run_timing` summed `filter_map(|stage| stage.timing)`, and
`stage.timing` is only written at finalization. Wall time ticked live off
`start_time`; active time did not tick at all.
Stage projections now accumulate brackets from the event log:
- Closing an inference bracket folds its span into `live_inference_ms`
instead of discarding it, including across retries, matching the
in-process stopwatch.
- Tool calls open a batch on the first outstanding call and close it when
the last one drains, so tools running concurrently within a turn count
once — the same span `execute_tool_calls` is bracketed by. Summing
per-call durations would over-count parallel tool use. Subagent tool
events are excluded; they run inside the root call's span already.
- `StageProjection::live_timing(now)` composes accumulators with any open
bracket, per handler: agent stages use the brackets, prompt and command
stages count elapsed time as inference and tool respectively, and
handlers that wait on a human, timer, condition, or child branches
report zero.
Active is clamped to wall per stage. A worker killed mid-turn leaves its
bracket open forever, and without the clamp it would tick up unbounded.
The clamp does not need to detect the dead worker: a stage cannot have been
active longer than it has existed. `watchdog.timeout` remains the authority
on whether a run is stuck. The clamp is deliberately not applied at run
level, where concurrent branches can legitimately sum past run wall time.
Timing is derived from events rather than emitted by the worker, so this
needs no event-schema change and applies to runs already stored.
`StageProjection.timing` keeps its terminal-only meaning, and the
authoritative breakdown still replaces the live estimate at terminal
events.
The billing endpoint had the same hole behind its `wall_only` fallback:
running stages reported zero inference/tool/active. Not visible in the
product, which renders only `wall_time_ms`, but wrong for any other
consumer of `GET /runs/{id}/billing`.
Parallel branch stages lose their breakdown permanently, even after
completion, because `parallel.branch.completed` carries only `duration_ms`.
That is a separate data-loss bug, tracked in #644.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A tab left open across a deploy keeps running the previous build's
JavaScript indefinitely. index.html is fetched only on a full page load,
all later navigation is client-side, and hashed bundles are served
`immutable`, so nothing reveals that the code is stale. This produced a
false-positive bug report where two correctly-deployed fixes appeared to
be missing.
Publishes a build id and offers a reload when the running document falls
behind. The toast never reloads on its own; the only automatic reload is
recovery from a chunk that no longer exists.
Build id derivation
-------------------
The obvious approach — hash the emitted asset filenames, which already
embed content hashes — does not work: Bun's minified identifier naming is
not deterministic. Building an unchanged tree twice produces byte-different
output roughly one run in three (same length, ~100k differing bytes, all of
it mangled names). Output hashes therefore move with no source change,
which would fire the toast on redeploys of identical code and train people
to ignore it.
The id is instead derived from the bundle's source inputs, so it changes if
and only if something we control changed. Verified stable across eight
consecutive builds while the entry hash flipped between both variants.
This non-determinism also means two builds of the same commit embed
different bytes into the server binary, which is worth addressing
separately for reproducible builds.
Detection
---------
SWR with `refreshInterval` + `revalidateOnFocus`, per the repo's React
effects policy. SWR does not poll while the document is hidden, so
background tabs stay quiet without extra gating. Unknown state on either
side — missing meta tag, failed fetch, 503 during a dev rebuild — never
produces a prompt.
Stylesheet hashing
------------------
Tailwind's output was stable-named and therefore served `no-cache`, letting
a tab revalidate into new CSS while running old JS. Tailwind purges unused
classes per build, so classes the old bundle still emits could silently
lose their styles. It is now content-hashed and moves with the build.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
agent.message already carries a `reasoning` property with the model's own
summary and its verbatim trace, and the generated client already types it.
The web app just never read it.
Read it onto the assistant turn and render it in the details panel, after
the message and before the metrics. A trace can run thousands of characters,
so leading with one would push the message the user clicked on below the
fold. Text over 280 characters collapses to a preview with a "Show all"
toggle, matching ChatUserCard's disclosure pattern.
Providers disclose one field or the other or both, so a trace with no
summary is labeled just "Reasoning" rather than "Reasoning trace" — that is
the common Anthropic thinking case, and the bare label reads better when
there is nothing to contrast it with. Both fields render as preformatted
text: reasoning is raw model output, not authored Markdown, and parsing it
would eat the line breaks that are part of what it says.
Adding the field to the assistant turn broke six existing toEqual fixtures
that assert whole turn objects; they now expect `reasoning: null`.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The prompt bubble is `w-fit max-w-[85%]`, so its width is measured
intrinsically and only then clamped. `items-start` left the inner content
wrapper intrinsically sized too, so it resolved against the available space
from before the clamp — the full column width — and kept that measurement
after the bubble shrank. The text laid out at 100% of the column while the
background painted at 85%, spilling out the right side.
Give the wrapper `w-full` so it fills the bubble's resolved width instead of
measuring itself. Short prompts still hug their content: a percentage-width
child contributes its content size during intrinsic sizing, so the bubble
measures the same and only the final wrap width changes. The expand button
keeps hugging its label as a separate flex child.
Also break long words in the collapsed preview. That is a separate overflow
path: the preview is raw prompt text under `whitespace-pre-wrap`, where an
unbreakable path or URL would spill even at the correct width.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A text-free agent message marks the boundary between two batches of tool
calls, so it stays in the turn stream to keep those batches as separate
"N tool calls" chips. But it rendered an empty prose div, which still took
a slot in the gap-4 column and doubled the vertical space between the chips
on either side of it.
Render nothing for those turns instead. The final assistant turn still
renders when it carries a token/duration footer, even with no text, so the
completed-stage metrics are unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Chat is the more useful first view for agent stages, so open there instead
of Thread. Only agent stages offer "chat" in availableTabs; every other
renderer already falls back to "primary", so this leaves Logs/Q&A/Decision
and the rest unchanged.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The "Model request · waiting on <model>" readout sat directly above the
Chat/Thread/Debug toolbar and appeared and disappeared as requests opened
and closed, shifting the toolbar underneath it.
Drops the StageInferenceIndicator component and everything that existed
only to feed it: the inference/runSettled prop threading through
RunStages, and StageActivity's watchdogTimedOut field. The watchdog.timeout
event now falls through to the same ignore path it always would have, since
it was never in STAGE_ACTIVITY_EVENT_TYPES.
The run-events invalidations for watchdog.timeout and agent.llm.* stay:
they still refresh stage events for the Debug tab and run state for the
insights sidebar.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
During a long LLM turn the durable event stream was silent: between
`agent.tool.completed` and the next `agent.message` nothing was emitted,
so "the model is generating" and "the worker is wedged" were
indistinguishable from the run store, SSE, or the UI.
The signal already existed. `AssistantTextStart` fired at exactly the
right point — after `build_request()`, after compaction, immediately
before the stream opens — then was classified as streaming noise and
thrown away. This promotes it rather than inventing a new one.
Two events, each asserting only what is provable when it is emitted:
- `agent.llm.started` carries the *requested* provider/model. No usage,
no cost, no context window: none of it exists yet, and failover can
re-target, so `agent.message` stays authoritative for what answered.
- `agent.llm.first_output` is edge-triggered on the first output of an
attempt and names what arrived. `ToolCall` is required, not optional:
a turn that opens with a tool call produces no text or reasoning
delta, so a latch keyed on those two would stay silent for exactly
the tool-heavy rounds where liveness matters most.
`agent.llm.retry` now also fires on the one previously invisible
mid-turn path — a stream that ends without a finish event, which
replays the turn and discards its output with nothing to show for it.
Its `attempt` field was already fed by two independent counters, so an
optional `phase` (open | consume) names which loop it counts.
`StageProjection.inference` projects the open bracket. `Some` means
"the event log contains an unclosed inference bracket", not "the model
is computing now" — a SIGKILLed worker leaves it open, which is the
truthful statement of what we know, and `watchdog.timeout` remains the
authority on actually-stuck.
The close is the subtle part. Terminal cancel and wall-clock timeout
tear the session down through `discard_session` without emitting a
message, error, or interrupt, so a session-lifecycle backstop is
required. It has to be `agent.session.ended`, not
`agent.session.deactivated`: deactivation is emitted by `lease.release()`
*before* the forwarder drains queued agent events, so a queued
`agent.llm.started` can arrive after it and re-open the bracket. But
`agent.session.ended` carries no stage identity, so the close takes
ordering from the event and identity from the projection, scanning for
brackets the ending session opened. A normal stage lookup there finds
no target and silently no-ops.
Presentation states what the log proves and nothing more: no progress
bar or ETA (no completion estimate exists), "reasoning" only when the
provider sent reasoning output, elapsed counted since the request
opened, and no live animation once the run is terminal.
Scope is session-backed agent stages. One-shot completions call
`client.complete` directly and never build a session; covering them
means moving the emit point into `fabro-llm`, filed as a follow-up.
`agent.output.start` was never persisted — it existed in a name map,
an `unreachable!` arm, and docs — so the rename carries no migration
risk. Corrects `events.md`, which documented it as a real emitted
event, and the v2 proposal, which mapped it to `message.part.started`
despite it firing before the request opens.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Empty-text `agent.message` events were discarded, erasing the boundaries
between batches of tool calls. Eight short shell calls issued across five
model responses collapsed into one `Bash x8` group whose DNA bar spanned
the model-response gaps between them, showing a misleading six-minute
duration. Filtering could recreate the same artificial adjacency.
- Always emit an assistant turn for `agent.message`, carrying
`tool_call_count` so a text-free response renders as
"Requested N tool calls" instead of a blank row.
- Derive grouping and DNA timing from the complete turn stream, then
apply kind/search filters as a pure visibility pass over display
items. Hiding a tool can no longer inflate an adjacent Agent bar, and
hiding an Agent can no longer merge the tool groups on either side.
- Give a tool group the wall-clock envelope of its children (earliest
start to latest end) rather than the sum of their durations or the
span to the last array element. Row, details header, DNA bar, and
tooltip all read the same values.
- Advance the DNA previous-activity cursor by the maximum observed end
so out-of-order or overlapping completions cannot move it backward.
Frontend only: no event, persistence, or API schema changes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds a Chat tab to agent stage pages alongside Thread and Debug, styled
after the Ask Fabro sidebar: agent messages render as first-class chat
bubbles (the narration between tool batches is the content that matters),
the stage prompt is a collapsed user-side card, and each run of
consecutive tool calls collapses to a wrench-icon count chip. While the
stage is running, in-flight tool calls (agent.tool.started without a
completed event) show as a live spinner line with the tool name and input
preview — data the Thread view drops today.
Thread remains the default tab; Chat becomes the default only after
production testing.
Also fixes the demo dataset: detect-drift carries the agent-flavored
stage events (prompt, agent messages, tool calls) but was labeled a
command stage, so its Thread/Chat views were unreachable in demo mode.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Resolves conflicts with the shared-checkout parallel rewrite (#607) and the
cached-run/billing dedup (de60eb900):
- handler/parallel.rs: rebuilt on main's shared-checkout version. Branch
ordinals are still reserved inside the branch task right before
ParallelBranchStarted (with graph_visit/resumed_from_stage_id), and the
reserved StageScope is shared with post-await error paths via a OnceLock
slot instead of main's dispatch-time visit=1 scope, so completion events
are never emitted under a guessed ordinal.
- billing.rs: keep this branch's run_stage_from_projection (RunStage grew
graph_visit/resumed_from_stage_id and a typed id), adopt main's
state.cached_run() and drop the removed run_stage_from_stage_id import.
- run_projection.rs: adopt main's typed parallel_results
(Option<Vec<ParallelBranchResult>>).
- run_event/misc.rs: union of both sides' imports.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
