mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-09-06 08:18:58 +00:00
parent
136abbb85c
commit
613b392d6e
6 changed files with 1321 additions and 19 deletions
279
run.json
279
run.json
File diff suppressed because one or more lines are too long
711
stages/006-simplify_opus@1/diff.patch
Normal file
711
stages/006-simplify_opus@1/diff.patch
Normal file
|
|
@ -0,0 +1,711 @@
|
|||
diff --git a/apps/fabro-web/app/components/stage-sidebar.tsx b/apps/fabro-web/app/components/stage-sidebar.tsx
|
||||
index b72ee77b..36d1d5d0 100644
|
||||
--- a/apps/fabro-web/app/components/stage-sidebar.tsx
|
||||
+++ b/apps/fabro-web/app/components/stage-sidebar.tsx
|
||||
@@ -1,4 +1,4 @@
|
||||
-import { useState, useEffect, useRef, type ComponentType } from "react";
|
||||
+import { useEffect, useRef, type ComponentType } from "react";
|
||||
import { Link } from "react-router";
|
||||
import type { StageState } from "@qltysh/fabro-api-client";
|
||||
import {
|
||||
@@ -12,6 +12,7 @@ import {
|
||||
import { Bars3BottomLeftIcon, DocumentTextIcon, MapIcon } from "@heroicons/react/24/outline";
|
||||
import { formatDurationSecs } from "../lib/format";
|
||||
import { ACTIVE_STAGE_STATES } from "../lib/stage-sidebar";
|
||||
+import { useTickingNow } from "../lib/time";
|
||||
|
||||
export interface Stage {
|
||||
id: string;
|
||||
@@ -42,7 +43,6 @@ interface StageSidebarProps {
|
||||
export function StageSidebar({ stages, runId, selectedStageId, activeLink }: StageSidebarProps) {
|
||||
// Track when we first observed each running stage (for ticking timer)
|
||||
const runningStartRef = useRef<Map<string, number>>(new Map());
|
||||
- const [, setTick] = useState(0);
|
||||
|
||||
// Track start times for running stages
|
||||
useEffect(() => {
|
||||
@@ -62,16 +62,13 @@ export function StageSidebar({ stages, runId, selectedStageId, activeLink }: Sta
|
||||
}, [stages]);
|
||||
|
||||
// Tick every second while any stage is running
|
||||
- useEffect(() => {
|
||||
- if (!stages.some((s) => ACTIVE_STAGE_STATES.has(s.status))) return;
|
||||
- const interval = setInterval(() => setTick((t) => t + 1), 1000);
|
||||
- return () => clearInterval(interval);
|
||||
- }, [stages]);
|
||||
+ const hasActive = stages.some((s) => ACTIVE_STAGE_STATES.has(s.status));
|
||||
+ const now = useTickingNow(hasActive);
|
||||
|
||||
function stageDuration(stage: Stage): string {
|
||||
if (ACTIVE_STAGE_STATES.has(stage.status)) {
|
||||
const start = runningStartRef.current.get(stage.id);
|
||||
- if (start) return formatDurationSecs(Math.floor((Date.now() - start) / 1000));
|
||||
+ if (start) return formatDurationSecs(Math.floor((now - start) / 1000));
|
||||
return "0s";
|
||||
}
|
||||
return stage.duration;
|
||||
@@ -156,4 +153,4 @@ export function StageSidebar({ stages, runId, selectedStageId, activeLink }: Sta
|
||||
</div>
|
||||
</nav>
|
||||
);
|
||||
-}
|
||||
+}
|
||||
\ No newline at end of file
|
||||
diff --git a/apps/fabro-web/app/lib/stage-sidebar.ts b/apps/fabro-web/app/lib/stage-sidebar.ts
|
||||
index 747c6a70..1423ec2c 100644
|
||||
--- a/apps/fabro-web/app/lib/stage-sidebar.ts
|
||||
+++ b/apps/fabro-web/app/lib/stage-sidebar.ts
|
||||
@@ -1,13 +1,22 @@
|
||||
-import type { PaginatedRunStageList, StageState } from "@qltysh/fabro-api-client";
|
||||
+import { StageState } from "@qltysh/fabro-api-client";
|
||||
+import type { PaginatedRunStageList } from "@qltysh/fabro-api-client";
|
||||
|
||||
import type { Stage } from "../components/stage-sidebar";
|
||||
import { isVisibleStage } from "../data/runs";
|
||||
import { formatDurationSecs } from "./format";
|
||||
|
||||
-export const ACTIVE_STAGE_STATES: ReadonlySet<StageState> = new Set(["running", "retrying"]);
|
||||
+export const ACTIVE_STAGE_STATES: ReadonlySet<StageState> = new Set([
|
||||
+ StageState.RUNNING,
|
||||
+ StageState.RETRYING,
|
||||
+]);
|
||||
+export const IN_FLIGHT_STAGE_STATES: ReadonlySet<StageState> = new Set([
|
||||
+ StageState.PENDING,
|
||||
+ StageState.RUNNING,
|
||||
+ StageState.RETRYING,
|
||||
+]);
|
||||
export const SUCCEEDED_STAGE_STATES: ReadonlySet<StageState> = new Set([
|
||||
- "succeeded",
|
||||
- "partially_succeeded",
|
||||
+ StageState.SUCCEEDED,
|
||||
+ StageState.PARTIALLY_SUCCEEDED,
|
||||
]);
|
||||
|
||||
export function mapRunStagesToSidebarStages(
|
||||
@@ -24,4 +33,4 @@ export function mapRunStagesToSidebarStages(
|
||||
? formatDurationSecs(stage.duration_secs)
|
||||
: "--",
|
||||
}));
|
||||
-}
|
||||
+}
|
||||
\ No newline at end of file
|
||||
diff --git a/apps/fabro-web/app/lib/time.ts b/apps/fabro-web/app/lib/time.ts
|
||||
index 1d8a818d..1810ce94 100644
|
||||
--- a/apps/fabro-web/app/lib/time.ts
|
||||
+++ b/apps/fabro-web/app/lib/time.ts
|
||||
@@ -1,3 +1,20 @@
|
||||
+import { useEffect, useState } from "react";
|
||||
+
|
||||
+/**
|
||||
+ * Re-renders the calling component every `intervalMs` milliseconds while
|
||||
+ * `active` is true, returning the current `Date.now()` value at each tick.
|
||||
+ * Returns the captured value when paused, so renders are stable.
|
||||
+ */
|
||||
+export function useTickingNow(active: boolean, intervalMs = 1000): number {
|
||||
+ const [now, setNow] = useState(() => Date.now());
|
||||
+ useEffect(() => {
|
||||
+ if (!active) return;
|
||||
+ const interval = setInterval(() => setNow(Date.now()), intervalMs);
|
||||
+ return () => clearInterval(interval);
|
||||
+ }, [active, intervalMs]);
|
||||
+ return now;
|
||||
+}
|
||||
+
|
||||
function relativeTime(seconds: number, past: boolean): string {
|
||||
if (seconds < 60) return past ? "just now" : "in <1m";
|
||||
const minutes = Math.floor(seconds / 60);
|
||||
@@ -21,4 +38,3 @@ export function timeAgo(iso: string): string {
|
||||
export function timeUntil(iso: string): string {
|
||||
return relativeTime(Math.floor((new Date(iso).getTime() - Date.now()) / 1000), false);
|
||||
}
|
||||
-
|
||||
diff --git a/apps/fabro-web/app/routes/run-billing.tsx b/apps/fabro-web/app/routes/run-billing.tsx
|
||||
index 74c712fd..959e9ef9 100644
|
||||
--- a/apps/fabro-web/app/routes/run-billing.tsx
|
||||
+++ b/apps/fabro-web/app/routes/run-billing.tsx
|
||||
@@ -1,9 +1,11 @@
|
||||
-import { useEffect, useState } from "react";
|
||||
+import { useMemo } from "react";
|
||||
|
||||
import { EmptyState } from "../components/state";
|
||||
import { formatDurationSecs } from "../lib/format";
|
||||
import { useRunBilling } from "../lib/queries";
|
||||
-import type { RunBilling } from "@qltysh/fabro-api-client";
|
||||
+import { IN_FLIGHT_STAGE_STATES } from "../lib/stage-sidebar";
|
||||
+import { useTickingNow } from "../lib/time";
|
||||
+import type { RunBilling, RunBillingStage, StageState } from "@qltysh/fabro-api-client";
|
||||
|
||||
const EMPTY_VALUE = "—";
|
||||
|
||||
@@ -16,8 +18,8 @@ function formatUsdMicros(usdMicros?: number | null) {
|
||||
return usdMicros == null ? EMPTY_VALUE : `$${(usdMicros / 1_000_000).toFixed(2)}`;
|
||||
}
|
||||
|
||||
-function isInFlightState(state: string | null | undefined): boolean {
|
||||
- return state === "running" || state === "retrying" || state === "pending";
|
||||
+function isInFlight(stage: RunBillingStage): boolean {
|
||||
+ return stage.state != null && IN_FLIGHT_STAGE_STATES.has(stage.state as StageState);
|
||||
}
|
||||
|
||||
interface MappedStageRow {
|
||||
@@ -27,129 +29,85 @@ interface MappedStageRow {
|
||||
outputTokens: number | null;
|
||||
runtimeSecs: number;
|
||||
totalUsdMicros: number | null | undefined;
|
||||
- inFlight: boolean;
|
||||
- startedAt: string | null | undefined;
|
||||
}
|
||||
|
||||
-interface MappedBilling {
|
||||
- rows: MappedStageRow[];
|
||||
- totalRuntimeSecs: number;
|
||||
- totalUsdMicros: number | null | undefined;
|
||||
- totalInput: number | null;
|
||||
- totalOutput: number | null;
|
||||
- modelBreakdown: {
|
||||
- model: string;
|
||||
- stages: number;
|
||||
- inputTokens: number;
|
||||
- outputTokens: number;
|
||||
- totalUsdMicros: number | null | undefined;
|
||||
- }[];
|
||||
- modelStageCount: number;
|
||||
- hasInFlight: boolean;
|
||||
-}
|
||||
-
|
||||
-function mapBilling(billing: RunBilling | undefined, now: number): MappedBilling {
|
||||
- if (!billing) {
|
||||
- return {
|
||||
- rows: [],
|
||||
- totalRuntimeSecs: 0,
|
||||
- totalUsdMicros: undefined,
|
||||
- totalInput: null,
|
||||
- totalOutput: null,
|
||||
- modelBreakdown: [],
|
||||
- modelStageCount: 0,
|
||||
- hasInFlight: false,
|
||||
- };
|
||||
- }
|
||||
-
|
||||
- let hasInFlight = false;
|
||||
- const rows: MappedStageRow[] = billing.stages.map((stage) => {
|
||||
- const hasModel = stage.model != null;
|
||||
- const inFlight = isInFlightState(stage.state);
|
||||
- if (inFlight) hasInFlight = true;
|
||||
-
|
||||
- let runtimeSecs = stage.runtime_secs;
|
||||
- if (inFlight && stage.started_at) {
|
||||
- const startedMs = new Date(stage.started_at).getTime();
|
||||
- if (Number.isFinite(startedMs)) {
|
||||
- runtimeSecs = Math.max(0, (now - startedMs) / 1000);
|
||||
- }
|
||||
+function liveRuntimeSecs(stage: RunBillingStage, now: number): number {
|
||||
+ if (stage.started_at) {
|
||||
+ const startedMs = new Date(stage.started_at).getTime();
|
||||
+ if (Number.isFinite(startedMs)) {
|
||||
+ return Math.max(0, (now - startedMs) / 1000);
|
||||
}
|
||||
+ }
|
||||
+ return stage.runtime_secs;
|
||||
+}
|
||||
|
||||
- return {
|
||||
- stage: stage.stage.name,
|
||||
- model: stage.model?.id ?? null,
|
||||
- inputTokens: hasModel ? stage.billing.input_tokens : null,
|
||||
- outputTokens: hasModel
|
||||
- ? stage.billing.output_tokens + stage.billing.reasoning_tokens
|
||||
- : null,
|
||||
- runtimeSecs,
|
||||
- totalUsdMicros: stage.billing.total_usd_micros,
|
||||
- inFlight,
|
||||
- startedAt: stage.started_at,
|
||||
- };
|
||||
- });
|
||||
-
|
||||
- // While ticking, derive total runtime from the displayed row runtimes so the
|
||||
- // footer updates in lock-step with the in-flight row(s). Otherwise trust the
|
||||
- // server's authoritative total.
|
||||
- const totalRuntimeSecs = hasInFlight
|
||||
- ? rows.reduce((sum, row) => sum + row.runtimeSecs, 0)
|
||||
- : billing.totals.runtime_secs;
|
||||
-
|
||||
- const hasLlmStages = billing.by_model.length > 0;
|
||||
- const totalInput = hasLlmStages ? billing.totals.input_tokens : null;
|
||||
- const totalOutput = hasLlmStages
|
||||
- ? billing.totals.output_tokens + billing.totals.reasoning_tokens
|
||||
- : null;
|
||||
- const totalUsdMicros = billing.totals.total_usd_micros;
|
||||
- const modelBreakdown = billing.by_model
|
||||
- .map((entry) => ({
|
||||
- model: entry.model.id,
|
||||
- stages: entry.stages,
|
||||
- inputTokens: entry.billing.input_tokens,
|
||||
- outputTokens: entry.billing.output_tokens + entry.billing.reasoning_tokens,
|
||||
- totalUsdMicros: entry.billing.total_usd_micros,
|
||||
- }))
|
||||
- .sort((a, b) => (b.totalUsdMicros ?? -1) - (a.totalUsdMicros ?? -1));
|
||||
- const modelStageCount = modelBreakdown.reduce((sum, row) => sum + row.stages, 0);
|
||||
-
|
||||
+function mapStageRow(stage: RunBillingStage, runtimeSecs: number): MappedStageRow {
|
||||
+ const hasModel = stage.model != null;
|
||||
return {
|
||||
- rows,
|
||||
- totalRuntimeSecs,
|
||||
- totalUsdMicros,
|
||||
- totalInput,
|
||||
- totalOutput,
|
||||
- modelBreakdown,
|
||||
- modelStageCount,
|
||||
- hasInFlight,
|
||||
+ stage: stage.stage.name,
|
||||
+ model: stage.model?.id ?? null,
|
||||
+ inputTokens: hasModel ? stage.billing.input_tokens : null,
|
||||
+ outputTokens: hasModel
|
||||
+ ? stage.billing.output_tokens + stage.billing.reasoning_tokens
|
||||
+ : null,
|
||||
+ runtimeSecs,
|
||||
+ totalUsdMicros: stage.billing.total_usd_micros,
|
||||
};
|
||||
}
|
||||
|
||||
export default function RunBilling({ params }: { params: { id: string } }) {
|
||||
const billingQuery = useRunBilling(params.id);
|
||||
-
|
||||
- // Tick state for live runtime computation. Re-rendered every second only
|
||||
- // while at least one stage is in-flight.
|
||||
- const [now, setNow] = useState(() => Date.now());
|
||||
const billing = billingQuery.data;
|
||||
- const hasInFlight = billing?.stages.some((stage) => isInFlightState(stage.state)) ?? false;
|
||||
+ const hasInFlight = billing?.stages.some(isInFlight) ?? false;
|
||||
+
|
||||
+ // Tick once per second only while a stage is in-flight.
|
||||
+ const now = useTickingNow(hasInFlight);
|
||||
+
|
||||
+ // Completed rows don't depend on `now`; memoize them by `billing` so we
|
||||
+ // don't reallocate them every tick.
|
||||
+ const completedRows = useMemo<MappedStageRow[]>(() => {
|
||||
+ if (!billing) return [];
|
||||
+ return billing.stages.map((stage) => mapStageRow(stage, stage.runtime_secs));
|
||||
+ }, [billing]);
|
||||
+
|
||||
+ // The model breakdown is server-derived and stable across ticks too.
|
||||
+ const modelBreakdown = useMemo(() => {
|
||||
+ if (!billing) return [];
|
||||
+ return billing.by_model
|
||||
+ .map((entry) => ({
|
||||
+ model: entry.model.id,
|
||||
+ stages: entry.stages,
|
||||
+ inputTokens: entry.billing.input_tokens,
|
||||
+ outputTokens: entry.billing.output_tokens + entry.billing.reasoning_tokens,
|
||||
+ totalUsdMicros: entry.billing.total_usd_micros,
|
||||
+ }))
|
||||
+ .sort((a, b) => (b.totalUsdMicros ?? -1) - (a.totalUsdMicros ?? -1));
|
||||
+ }, [billing]);
|
||||
+
|
||||
+ // Re-derive only the in-flight rows on each tick; everything else stays put.
|
||||
+ const rows = useMemo<MappedStageRow[]>(() => {
|
||||
+ if (!billing) return [];
|
||||
+ if (!hasInFlight) return completedRows;
|
||||
+ return billing.stages.map((stage, idx) =>
|
||||
+ isInFlight(stage)
|
||||
+ ? mapStageRow(stage, liveRuntimeSecs(stage, now))
|
||||
+ : completedRows[idx],
|
||||
+ );
|
||||
+ }, [billing, completedRows, hasInFlight, now]);
|
||||
|
||||
- useEffect(() => {
|
||||
- if (!hasInFlight) return;
|
||||
- const interval = setInterval(() => setNow(Date.now()), 1000);
|
||||
- return () => clearInterval(interval);
|
||||
- }, [hasInFlight]);
|
||||
+ // While ticking, sum the displayed row runtimes so the footer updates in
|
||||
+ // lock-step. Otherwise trust the server's authoritative total.
|
||||
+ const totalRuntimeSecs = hasInFlight
|
||||
+ ? rows.reduce((sum, row) => sum + row.runtimeSecs, 0)
|
||||
+ : (billing?.totals.runtime_secs ?? 0);
|
||||
|
||||
- const {
|
||||
- rows,
|
||||
- totalRuntimeSecs,
|
||||
- totalUsdMicros,
|
||||
- totalInput,
|
||||
- totalOutput,
|
||||
- modelBreakdown,
|
||||
- modelStageCount,
|
||||
- } = mapBilling(billing, now);
|
||||
+ const hasLlmStages = (billing?.by_model.length ?? 0) > 0;
|
||||
+ const totalInput = hasLlmStages ? (billing?.totals.input_tokens ?? null) : null;
|
||||
+ const totalOutput = hasLlmStages && billing
|
||||
+ ? billing.totals.output_tokens + billing.totals.reasoning_tokens
|
||||
+ : null;
|
||||
+ const totalUsdMicros = billing?.totals.total_usd_micros;
|
||||
+ const modelStageCount = modelBreakdown.reduce((sum, row) => sum + row.stages, 0);
|
||||
|
||||
if (!rows.length) {
|
||||
return (
|
||||
@@ -265,4 +223,4 @@ export default function RunBilling({ params }: { params: { id: string } }) {
|
||||
) : null}
|
||||
</div>
|
||||
);
|
||||
-}
|
||||
+}
|
||||
\ No newline at end of file
|
||||
diff --git a/apps/fabro-web/app/routes/run-stages.tsx b/apps/fabro-web/app/routes/run-stages.tsx
|
||||
index 6d179d70..ab70b98c 100644
|
||||
--- a/apps/fabro-web/app/routes/run-stages.tsx
|
||||
+++ b/apps/fabro-web/app/routes/run-stages.tsx
|
||||
@@ -40,6 +40,7 @@ import type { Stage } from "../components/stage-sidebar";
|
||||
import { EmptyState } from "../components/state";
|
||||
import { CopyButton } from "../components/ui";
|
||||
import { formatDurationSecs } from "../lib/format";
|
||||
+import { useTickingNow } from "../lib/time";
|
||||
import { fetchRunCommandLog, useRunEventsList, useRunStageTurns, useRunStages } from "../lib/queries";
|
||||
import { mapRunStagesToSidebarStages } from "../lib/stage-sidebar";
|
||||
import { getNumber, getString, type UnknownRecord } from "../lib/unknown";
|
||||
@@ -575,7 +576,6 @@ function RunningStageDuration({
|
||||
const [startedAt, setStartedAt] = useState<number | null>(() =>
|
||||
isRunning ? Date.now() : null,
|
||||
);
|
||||
- const [, setTick] = useState(0);
|
||||
|
||||
useEffect(() => {
|
||||
setStartedAt((current) => {
|
||||
@@ -584,14 +584,10 @@ function RunningStageDuration({
|
||||
});
|
||||
}, [isRunning]);
|
||||
|
||||
- useEffect(() => {
|
||||
- if (!isRunning) return;
|
||||
- const interval = setInterval(() => setTick((tick) => tick + 1), 1000);
|
||||
- return () => clearInterval(interval);
|
||||
- }, [isRunning]);
|
||||
+ const now = useTickingNow(isRunning);
|
||||
|
||||
if (isRunning && startedAt) {
|
||||
- return formatDurationSecs(Math.floor((Date.now() - startedAt) / 1000));
|
||||
+ return formatDurationSecs(Math.floor((now - startedAt) / 1000));
|
||||
}
|
||||
return duration;
|
||||
}
|
||||
@@ -661,4 +657,4 @@ export default function RunStages() {
|
||||
</div>
|
||||
</div>
|
||||
);
|
||||
-}
|
||||
+}
|
||||
\ No newline at end of file
|
||||
diff --git a/lib/crates/fabro-api/tests/run_billing_stage_round_trip.rs b/lib/crates/fabro-api/tests/run_billing_stage_round_trip.rs
|
||||
index 5323e3dd..52fb4ece 100644
|
||||
--- a/lib/crates/fabro-api/tests/run_billing_stage_round_trip.rs
|
||||
+++ b/lib/crates/fabro-api/tests/run_billing_stage_round_trip.rs
|
||||
@@ -51,8 +51,8 @@ fn run_billing_stage_round_trips_terminal_row_with_started_at_and_state() {
|
||||
"state": "succeeded"
|
||||
});
|
||||
|
||||
- let stage: RunBillingStage = serde_json::from_value(value.clone())
|
||||
- .expect("terminal stage row should deserialize");
|
||||
+ let stage: RunBillingStage =
|
||||
+ serde_json::from_value(value.clone()).expect("terminal stage row should deserialize");
|
||||
assert!(stage.started_at.is_some());
|
||||
assert_eq!(stage.state, Some(StageState::Succeeded));
|
||||
assert_eq!(serde_json::to_value(stage).unwrap(), value);
|
||||
@@ -79,9 +79,9 @@ fn run_billing_stage_round_trips_in_flight_row() {
|
||||
"state": "running"
|
||||
});
|
||||
|
||||
- let stage: RunBillingStage = serde_json::from_value(value.clone())
|
||||
- .expect("in-flight stage row should deserialize");
|
||||
+ let stage: RunBillingStage =
|
||||
+ serde_json::from_value(value.clone()).expect("in-flight stage row should deserialize");
|
||||
assert!(stage.model.is_none());
|
||||
assert_eq!(stage.state, Some(StageState::Running));
|
||||
assert_eq!(serde_json::to_value(stage).unwrap(), value);
|
||||
-}
|
||||
\ No newline at end of file
|
||||
+}
|
||||
diff --git a/lib/crates/fabro-api/tests/stage_projection_round_trip.rs b/lib/crates/fabro-api/tests/stage_projection_round_trip.rs
|
||||
index 5ec8ab69..ad4dcb57 100644
|
||||
--- a/lib/crates/fabro-api/tests/stage_projection_round_trip.rs
|
||||
+++ b/lib/crates/fabro-api/tests/stage_projection_round_trip.rs
|
||||
@@ -46,4 +46,4 @@ fn assert_same_type<T: 'static, U: 'static>() {
|
||||
type_name::<T>(),
|
||||
type_name::<U>()
|
||||
);
|
||||
-}
|
||||
\ No newline at end of file
|
||||
+}
|
||||
diff --git a/lib/crates/fabro-server/src/demo/mod.rs b/lib/crates/fabro-server/src/demo/mod.rs
|
||||
index c0c9858b..a51cb405 100644
|
||||
--- a/lib/crates/fabro-server/src/demo/mod.rs
|
||||
+++ b/lib/crates/fabro-server/src/demo/mod.rs
|
||||
@@ -1709,4 +1709,4 @@ session_sandboxes = false
|
||||
})
|
||||
.clone()
|
||||
}
|
||||
-}
|
||||
\ No newline at end of file
|
||||
+}
|
||||
diff --git a/lib/crates/fabro-server/src/server/handler/billing.rs b/lib/crates/fabro-server/src/server/handler/billing.rs
|
||||
index cb50b27d..f128aadc 100644
|
||||
--- a/lib/crates/fabro-server/src/server/handler/billing.rs
|
||||
+++ b/lib/crates/fabro-server/src/server/handler/billing.rs
|
||||
@@ -24,17 +24,17 @@ pub(super) fn routes() -> Router<Arc<AppState>> {
|
||||
/// node's first appearance in the event log. This produces the same A, B
|
||||
/// order for an A → B → A loop that finalize produces.
|
||||
struct DedupedStage<'a> {
|
||||
- node_id: String,
|
||||
- stage: &'a StageProjection,
|
||||
- sort_key_first_event: u32,
|
||||
+ node_id: String,
|
||||
+ stage: &'a StageProjection,
|
||||
+ sort_key_first_event: u32,
|
||||
}
|
||||
|
||||
fn dedupe_by_node_id<'a>(
|
||||
stages: impl IntoIterator<Item = (&'a StageId, &'a StageProjection)>,
|
||||
) -> Vec<DedupedStage<'a>> {
|
||||
- let mut by_node: HashMap<String, (u32, u32, &'a StageProjection)> = HashMap::new();
|
||||
+ let mut by_node: HashMap<&'a str, (u32, u32, &'a StageProjection)> = HashMap::new();
|
||||
for (stage_id, stage) in stages {
|
||||
- let node_id = stage_id.node_id().to_string();
|
||||
+ let node_id = stage_id.node_id();
|
||||
let visit = stage_id.visit();
|
||||
let first_event = stage.first_event_seq.get();
|
||||
by_node
|
||||
@@ -54,7 +54,7 @@ fn dedupe_by_node_id<'a>(
|
||||
let mut deduped: Vec<DedupedStage<'a>> = by_node
|
||||
.into_iter()
|
||||
.map(|(node_id, (first_event, _visit, stage))| DedupedStage {
|
||||
- node_id,
|
||||
+ node_id: node_id.to_string(),
|
||||
stage,
|
||||
sort_key_first_event: first_event,
|
||||
})
|
||||
@@ -92,13 +92,16 @@ async fn list_run_stages(
|
||||
let now = Utc::now();
|
||||
let stages: Vec<RunStage> = dedupe_by_node_id(projection.iter_stages())
|
||||
.into_iter()
|
||||
- .map(|entry| RunStage {
|
||||
- id: entry.node_id.clone(),
|
||||
- name: entry.node_id.clone(),
|
||||
- status: entry.stage.effective_state(),
|
||||
- duration_secs: entry.stage.runtime_secs(now),
|
||||
- dot_id: Some(entry.node_id.clone()),
|
||||
- started_at: entry.stage.started_at,
|
||||
+ .map(|entry| {
|
||||
+ let DedupedStage { node_id, stage, .. } = entry;
|
||||
+ RunStage {
|
||||
+ id: node_id.clone(),
|
||||
+ name: node_id.clone(),
|
||||
+ status: stage.effective_state(),
|
||||
+ duration_secs: stage.runtime_secs(now),
|
||||
+ dot_id: Some(node_id),
|
||||
+ started_at: stage.started_at,
|
||||
+ }
|
||||
})
|
||||
.collect();
|
||||
|
||||
@@ -128,19 +131,16 @@ async fn get_run_billing(
|
||||
let now = Utc::now();
|
||||
|
||||
let mut by_model_totals = HashMap::<String, ModelBillingTotals>::new();
|
||||
- let mut billed_usages = Vec::new();
|
||||
let mut runtime_secs = 0.0_f64;
|
||||
let mut stages = Vec::new();
|
||||
|
||||
for entry in dedupe_by_node_id(projection.iter_stages()) {
|
||||
- let stage = entry.stage;
|
||||
- let node_id = entry.node_id;
|
||||
+ let DedupedStage { node_id, stage, .. } = entry;
|
||||
|
||||
let row_runtime = stage.runtime_secs(now).unwrap_or(0.0);
|
||||
runtime_secs += row_runtime;
|
||||
|
||||
let (billing, model) = if let Some(usage) = stage.usage.as_ref() {
|
||||
- billed_usages.push(usage.clone());
|
||||
let tokens = usage.tokens();
|
||||
let billing = BilledTokenCounts {
|
||||
cache_read_tokens: tokens.cache_read_tokens,
|
||||
@@ -151,9 +151,18 @@ async fn get_run_billing(
|
||||
total_tokens: tokens.total_tokens(),
|
||||
total_usd_micros: usage.total_usd_micros,
|
||||
};
|
||||
- let model_id = usage.model_id().to_string();
|
||||
- accumulate_model_billing(by_model_totals.entry(model_id.clone()).or_default(), usage);
|
||||
- (billing, Some(ModelReference { id: model_id }))
|
||||
+ let model_id = usage.model_id();
|
||||
+ let model_totals = match by_model_totals.get_mut(model_id) {
|
||||
+ Some(totals) => totals,
|
||||
+ None => by_model_totals.entry(model_id.to_string()).or_default(),
|
||||
+ };
|
||||
+ accumulate_model_billing(model_totals, usage);
|
||||
+ (
|
||||
+ billing,
|
||||
+ Some(ModelReference {
|
||||
+ id: model_id.to_string(),
|
||||
+ }),
|
||||
+ )
|
||||
} else {
|
||||
(BilledTokenCounts::default(), None)
|
||||
};
|
||||
@@ -171,7 +180,20 @@ async fn get_run_billing(
|
||||
});
|
||||
}
|
||||
|
||||
- let totals = BilledTokenCounts::from_billed_usage(&billed_usages);
|
||||
+ // Grand totals are the sum of the per-model totals we already accumulated.
|
||||
+ let mut totals = BilledTokenCounts::default();
|
||||
+ for model_totals in by_model_totals.values() {
|
||||
+ totals.input_tokens += model_totals.billing.input_tokens;
|
||||
+ totals.output_tokens += model_totals.billing.output_tokens;
|
||||
+ totals.reasoning_tokens += model_totals.billing.reasoning_tokens;
|
||||
+ totals.cache_read_tokens += model_totals.billing.cache_read_tokens;
|
||||
+ totals.cache_write_tokens += model_totals.billing.cache_write_tokens;
|
||||
+ totals.total_tokens += model_totals.billing.total_tokens;
|
||||
+ if let Some(value) = model_totals.billing.total_usd_micros {
|
||||
+ *totals.total_usd_micros.get_or_insert(0) += value;
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
let by_model = by_model_totals
|
||||
.into_iter()
|
||||
.map(|(model, totals)| BillingByModel {
|
||||
@@ -197,4 +219,4 @@ async fn get_run_billing(
|
||||
};
|
||||
|
||||
(StatusCode::OK, Json(response)).into_response()
|
||||
-}
|
||||
\ No newline at end of file
|
||||
+}
|
||||
diff --git a/lib/crates/fabro-server/src/server/tests.rs b/lib/crates/fabro-server/src/server/tests.rs
|
||||
index 6de28d70..47fd611c 100644
|
||||
--- a/lib/crates/fabro-server/src/server/tests.rs
|
||||
+++ b/lib/crates/fabro-server/src/server/tests.rs
|
||||
@@ -7462,4 +7462,4 @@ fn validate_github_slug_rejects_path_traversal_and_separators() {
|
||||
fn validate_github_slug_rejects_overlong() {
|
||||
let long = "a".repeat(40);
|
||||
assert!(super::validate_github_slug("owner", &long, 39).is_err());
|
||||
-}
|
||||
\ No newline at end of file
|
||||
+}
|
||||
diff --git a/lib/crates/fabro-server/tests/it/scenario/usage.rs b/lib/crates/fabro-server/tests/it/scenario/usage.rs
|
||||
index 264124bc..5f5f250e 100644
|
||||
--- a/lib/crates/fabro-server/tests/it/scenario/usage.rs
|
||||
+++ b/lib/crates/fabro-server/tests/it/scenario/usage.rs
|
||||
@@ -67,7 +67,7 @@ async fn run_billing_includes_completed_non_llm_stages() {
|
||||
assert_eq!(status, "succeeded");
|
||||
|
||||
let billing = run_billing(&app, &run_id).await;
|
||||
- assert_non_llm_billing(&billing, &["start"]);
|
||||
+ assert_non_llm_billing(&billing, &["exit", "start"]);
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -81,7 +81,7 @@ async fn run_billing_includes_completed_command_stages() {
|
||||
assert_eq!(status, "succeeded");
|
||||
|
||||
let billing = run_billing(&app, &run_id).await;
|
||||
- assert_non_llm_billing(&billing, &["echo_task", "start"]);
|
||||
+ assert_non_llm_billing(&billing, &["echo_task", "exit", "start"]);
|
||||
}
|
||||
|
||||
async fn run_billing(app: &axum::Router, run_id: &str) -> serde_json::Value {
|
||||
diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs
|
||||
index c9209859..6dc0efb6 100644
|
||||
--- a/lib/crates/fabro-store/src/run_state.rs
|
||||
+++ b/lib/crates/fabro-store/src/run_state.rs
|
||||
@@ -295,9 +295,7 @@ impl RunProjectionReducer for RunProjection {
|
||||
stage_id.visit(),
|
||||
first_event_seq(event.seq),
|
||||
);
|
||||
- stage.reset_for_new_attempt();
|
||||
- stage.started_at = Some(ts);
|
||||
- stage.state = Some(StageState::Running);
|
||||
+ stage.begin_attempt(ts);
|
||||
}
|
||||
EventBody::StageRetrying(_) => {
|
||||
let Some(stage) = stage_at_current_visit(self, stored, event.seq) else {
|
||||
@@ -1520,10 +1518,7 @@ mod tests {
|
||||
fn failed_props(duration_ms: u64) -> StageFailedProps {
|
||||
StageFailedProps {
|
||||
index: 0,
|
||||
- failure: Some(FailureDetail::new(
|
||||
- "boom",
|
||||
- FailureCategory::TransientInfra,
|
||||
- )),
|
||||
+ failure: Some(FailureDetail::new("boom", FailureCategory::TransientInfra)),
|
||||
will_retry: true,
|
||||
duration_ms,
|
||||
}
|
||||
@@ -1702,4 +1697,4 @@ mod tests {
|
||||
assert!(stage.completion.is_none());
|
||||
assert_eq!(stage.duration_ms, None);
|
||||
}
|
||||
-}
|
||||
\ No newline at end of file
|
||||
+}
|
||||
diff --git a/lib/crates/fabro-types/src/run_projection.rs b/lib/crates/fabro-types/src/run_projection.rs
|
||||
index 32cdbabc..b8aca23b 100644
|
||||
--- a/lib/crates/fabro-types/src/run_projection.rs
|
||||
+++ b/lib/crates/fabro-types/src/run_projection.rs
|
||||
@@ -137,43 +137,20 @@ impl StageProjection {
|
||||
StageState::Running | StageState::Retrying | StageState::Pending
|
||||
) {
|
||||
return self.started_at.map(|started| {
|
||||
- now.signed_duration_since(started)
|
||||
- .num_milliseconds()
|
||||
- .max(0) as f64
|
||||
- / 1000.0
|
||||
+ now.signed_duration_since(started).num_milliseconds().max(0) as f64 / 1000.0
|
||||
});
|
||||
}
|
||||
self.duration_ms.map(|ms| ms as f64 / 1000.0)
|
||||
}
|
||||
|
||||
- /// Reset every per-attempt result field. Called when a stage starts a
|
||||
- /// new attempt (or visit) so prior-attempt data does not leak into the
|
||||
- /// new attempt's projection.
|
||||
- ///
|
||||
- /// Preserves `first_event_seq` (identity / sort key) and leaves
|
||||
- /// `started_at` / `state` to be set by the caller immediately after.
|
||||
- pub fn reset_for_new_attempt(&mut self) {
|
||||
- self.completion = None;
|
||||
- self.duration_ms = None;
|
||||
- self.usage = None;
|
||||
- self.state = None;
|
||||
-
|
||||
- self.response = None;
|
||||
- self.prompt = None;
|
||||
- self.provider_used = None;
|
||||
- self.diff = None;
|
||||
-
|
||||
- self.script_invocation = None;
|
||||
- self.script_timing = None;
|
||||
- self.parallel_results = None;
|
||||
-
|
||||
- self.stdout = None;
|
||||
- self.stderr = None;
|
||||
- self.stdout_bytes = None;
|
||||
- self.stderr_bytes = None;
|
||||
- self.streams_separated = None;
|
||||
- self.live_streaming = None;
|
||||
- self.termination = None;
|
||||
+ /// Begin a new attempt (or visit) for this stage: clear every
|
||||
+ /// per-attempt field so prior-attempt data does not leak, then record
|
||||
+ /// `started_at` and `state = Running`. Preserves `first_event_seq`
|
||||
+ /// (identity / sort key).
|
||||
+ pub fn begin_attempt(&mut self, started_at: DateTime<Utc>) {
|
||||
+ *self = Self::new(self.first_event_seq);
|
||||
+ self.started_at = Some(started_at);
|
||||
+ self.state = Some(StageState::Running);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -268,4 +245,4 @@ impl RunProjection {
|
||||
}
|
||||
}
|
||||
}
|
||||
-}
|
||||
\ No newline at end of file
|
||||
+}
|
||||
6
stages/006-simplify_opus@1/status.json
Normal file
6
stages/006-simplify_opus@1/status.json
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
{
|
||||
"outcome": "succeeded",
|
||||
"notes": "Stage completed: simplify_opus",
|
||||
"failure_reason": null,
|
||||
"timestamp": "2026-05-04T21:00:33.411619Z"
|
||||
}
|
||||
311
stages/007-simplify_gpt@1/prompt.md
Normal file
311
stages/007-simplify_gpt@1/prompt.md
Normal file
|
|
@ -0,0 +1,311 @@
|
|||
Goal: # Billing & Stages: Read From Projection
|
||||
|
||||
## Context
|
||||
|
||||
The Billing tab on a running run omits the in-flight stage entirely, and the footer total runtime is frozen at the last server response.
|
||||
|
||||
Root cause: `GET /runs/{id}/billing` and `GET /runs/{id}/stages` (both in `lib/crates/fabro-server/src/server/handler/billing.rs`) bypass `RunProjection` and read `checkpoint.completed_nodes` + `checkpoint.node_outcomes` directly. The checkpoint only knows about *finished* nodes, so in-flight stages are invisible. `list_run_stages` had to grow a `next_node_id` workaround at `:113`; billing has no equivalent.
|
||||
|
||||
`RunProjection` is the canonical event-sourced read model. `StageStarted` already creates a `StageProjection` entry the moment a stage begins (`run_state.rs:289`). The projection just doesn't yet store `started_at`, completion duration, billing usage, or `state` (Retrying vs Running).
|
||||
|
||||
Goal: extend `StageProjection` with the missing event-derived fields, then collapse both handlers to thin views over `RunProjection.iter_stages()`. In-flight rows fall out for free. The frontend ticks runtime client-side using a server-supplied `started_at`.
|
||||
|
||||
Audit confirmed these are the only two read endpoints with the bypass pattern.
|
||||
|
||||
## Plan
|
||||
|
||||
### 1. Extend `StageProjection`
|
||||
|
||||
File: `lib/crates/fabro-types/src/run_projection.rs`
|
||||
|
||||
Add four fields to `StageProjection`:
|
||||
|
||||
```rust
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub started_at: Option<DateTime<Utc>>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub duration_ms: Option<u64>,
|
||||
#[serde(skip)] // server-internal; not on the wire
|
||||
pub usage: Option<BilledModelUsage>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub state: Option<StageState>,
|
||||
```
|
||||
|
||||
Why store `state` instead of deriving: the reducer needs to track `Retrying` (from `StageRetrying` events), which is not derivable from `completion` alone. Storing the field keeps the projection correct and removes the need for the existing `active_stage_state_from_events` event-replay (`billing.rs:19`). Use `Option<_>` so old serialized projections deserialize as `None` and can fall through a derivation helper.
|
||||
|
||||
Why `usage` is `#[serde(skip)]`: `BilledModelUsage` has no OpenAPI schema today (only `BilledTokenCounts` does, at `fabro-api.yaml:5756`). Modeling the full nested usage shape is out of scope for this PR, and `/runs/{id}/state` consumers can hit `/billing` if they need per-stage tokens. The billing handler reads `stage.usage` in-process to build `RunBillingStage.billing`. The field still survives in-process projection rebuild because `apply_event` reapplies it from `StageCompletedProps.billing` on every load.
|
||||
|
||||
Helper methods:
|
||||
|
||||
```rust
|
||||
pub fn effective_state(&self) -> StageState {
|
||||
self.state.unwrap_or_else(|| match &self.completion {
|
||||
Some(c) => StageState::from(c.outcome),
|
||||
None => StageState::Running,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn runtime_secs(&self, now: DateTime<Utc>) -> Option<f64> {
|
||||
// Live state ticks; only use stored duration_ms once terminal.
|
||||
// This handles retries safely: even if a previous failed attempt left
|
||||
// `duration_ms` set, the new `state = Running` makes us recompute live.
|
||||
let state = self.effective_state();
|
||||
if matches!(state, StageState::Running | StageState::Retrying | StageState::Pending) {
|
||||
return self.started_at.map(|started| {
|
||||
now.signed_duration_since(started)
|
||||
.num_milliseconds()
|
||||
.max(0) as f64
|
||||
/ 1000.0
|
||||
});
|
||||
}
|
||||
self.duration_ms.map(|ms| ms as f64 / 1000.0)
|
||||
}
|
||||
```
|
||||
|
||||
`effective_state` keeps old serialized projections working without a backfill.
|
||||
|
||||
Update `StageProjection::new` to default the four new fields to `None`.
|
||||
|
||||
### 2. Capture the new fields in the reducer
|
||||
|
||||
File: `lib/crates/fabro-store/src/run_state.rs`. The reducer already has `let ts = stored.ts` in scope at `:46`.
|
||||
|
||||
- `StageStarted` arm (`:289`): add a `StageProjection::reset_for_new_attempt(&mut self)` helper and call it after `stage_entry(...)`, then set `stage.started_at = Some(ts)` and `stage.state = Some(StageState::Running)`.
|
||||
|
||||
`reset_for_new_attempt` clears **every attempt-result field**, because all of them are repopulated by per-attempt lifecycle events (`run_state.rs:299, 306, 312, 324, 338, 344, 350, 359, 375`) and would otherwise leak prior-attempt data on retry:
|
||||
|
||||
- `completion`, `duration_ms`, `usage`, `state` (terminal data)
|
||||
- `response`, `prompt`, `provider_used`, `diff` (LLM/agent attempt data)
|
||||
- `script_invocation`, `script_timing`, `parallel_results` (handler attempt data)
|
||||
- `stdout`, `stderr`, `stdout_bytes`, `stderr_bytes`, `streams_separated`, `live_streaming`, `termination` (command-output attempt data)
|
||||
|
||||
The only fields preserved are `first_event_seq` (identity / sort key, set on first creation) and `started_at` / `state` which are written immediately after the reset. Without this reset, a retry with reused visit would leave `state = Running` alongside `completion.outcome = Failed` and prior `stdout`/`stderr` content — inconsistent projection state visible via `/runs/{id}/state`.
|
||||
- `StageCompleted` arm (`:312`): set `stage.duration_ms = Some(props.duration_ms)`, `stage.usage = props.billing.clone()`, `stage.state = Some(StageState::from(stage_outcome_from_props(props).status))`.
|
||||
- `StageFailed` arm (`:324`): set `stage.duration_ms = Some(props.duration_ms)` and `stage.state = Some(StageState::Failed)`.
|
||||
- `StageRetrying` arm: new — locate stage at current visit, set `stage.state = Some(StageState::Retrying)`. (No corresponding handler exists today.)
|
||||
|
||||
Add unit tests in the existing `#[cfg(test)] mod tests` block for each arm and one transition test (`StageStarted → StageFailed → StageRetrying → StageStarted` returns to `Running`).
|
||||
|
||||
### 3. Rewrite `get_run_billing`
|
||||
|
||||
File: `lib/crates/fabro-server/src/server/handler/billing.rs:128`
|
||||
|
||||
Replace the `checkpoint.completed_nodes` loop (`:179`) with:
|
||||
|
||||
1. Load `RunProjection` once (already done at `:140`).
|
||||
2. Capture `now: DateTime<Utc>` once.
|
||||
3. Collect `(StageId, &StageProjection)` from `projection.iter_stages()` into a `Vec`.
|
||||
4. Aggregate by `node_id` to align with finalized output (`fabro-workflow/src/pipeline/finalize.rs:113`):
|
||||
- **Order**: first occurrence wins. For each `node_id`, the sort key is the **minimum** `first_event_seq` across all of that node's visits (i.e. when the node first appeared in the event log).
|
||||
- **Data**: latest visit wins. The displayed row uses fields from the entry with the largest `visit` for that node_id.
|
||||
- This produces the same A, B order for an A→B→A loop that finalize produces. The current live handler iterates `checkpoint.completed_nodes: Vec<String>` directly and could emit duplicate rows for revisits; the new behavior collapses them, intentionally matching finalize.
|
||||
5. Sort the deduped rows by the per-node_id minimum `first_event_seq` from step 4.
|
||||
6. For each stage, build a `RunBillingStage`:
|
||||
- `stage`: `BillingStageRef { id, name = node_id }`.
|
||||
- `model`: from `stage.usage.as_ref().map(|u| ModelReference { id: u.model_id().to_string() })`.
|
||||
- `billing`: from `stage.usage` via the existing `BilledTokenCounts` shape; default if `None`.
|
||||
- `runtime_secs`: `stage.runtime_secs(now).unwrap_or(0.0)`.
|
||||
- `started_at`: `stage.started_at` (new field — see §5).
|
||||
- `state`: `stage.effective_state()` (new field — see §5).
|
||||
7. Totals: server-side total `runtime_secs` sums all rendered row runtimes (now includes the in-flight row's elapsed time). Tokens & cost via `BilledTokenCounts::from_billed_usage` over completed-stage usage — same as today.
|
||||
8. By-model breakdown: same as today, built from projection-derived usage list.
|
||||
|
||||
Drop the dependency on `fabro_workflow::extract_stage_durations_from_events` from this handler.
|
||||
|
||||
### 4. Rewrite `list_run_stages`
|
||||
|
||||
Same handler, `:38`.
|
||||
|
||||
Same shape as §3 for `RunStage`:
|
||||
|
||||
- Iterate `projection.iter_stages()`, dedupe by node_id with the same rule as §3 step 4: latest-visit data, sort by per-node_id minimum `first_event_seq`.
|
||||
- `RunStage { id, name, status: stage.effective_state(), duration_secs: stage.runtime_secs(now), dot_id: Some(node_id), started_at: stage.started_at }`.
|
||||
- Drop the `next_node_id` synthesis at `:113`.
|
||||
- Drop the live-vs-store fork at `:50–78`; the projection is updated as events are written, so a single `state.store.open_run_reader(...).state()` read suffices.
|
||||
- Delete `active_stage_state_from_events` at `:19` — no longer needed; `state` is on the projection.
|
||||
|
||||
### 5. OpenAPI: extend three schemas
|
||||
|
||||
File: `docs/public/api-reference/fabro-api.yaml`
|
||||
|
||||
- **`RunBillingStage`** (`:6610`): add optional `started_at: string (date-time)` and `state: $ref StageState`. Frontend uses `state` to detect in-flight rows.
|
||||
- **`RunStage`** (`:6316`): add optional `started_at: string (date-time)`. `status: StageState` already exists.
|
||||
- **`StageProjection`** (`:5279`): add optional `started_at`, `duration_ms`, and `state: StageState`. **Do not** add `usage` here — the field is `#[serde(skip)]` server-internal (see §1). `BilledModelUsage` is not currently an OpenAPI schema and modeling it would balloon this PR's surface; `/runs/{id}/state` consumers needing per-stage tokens hit `/billing` instead.
|
||||
|
||||
After editing: `cargo build -p fabro-api` regenerates Rust types; `cd lib/packages/fabro-api-client && bun run generate` regenerates the TS client.
|
||||
|
||||
### 6. Update demo fixtures
|
||||
|
||||
File: `lib/crates/fabro-server/src/demo/mod.rs`
|
||||
|
||||
- `RunStage` literals at `:1184, 1191, 1198, 1205` — add `started_at: None`.
|
||||
- `RunBillingStage` literals at `:1233, 1252, 1271, 1290` — add `started_at: None` and `state: StageState::Succeeded` (or appropriate per fixture).
|
||||
- Any `StageProjection` literals in tests/fixtures — search `rg "StageProjection \{"` and add the new optional fields (typically `..Default::default()` shape if used).
|
||||
|
||||
### 7. Frontend: invalidate on stage events + live tick
|
||||
|
||||
Files: `apps/fabro-web/app/lib/run-events.ts`, `apps/fabro-web/app/routes/run-billing.tsx`.
|
||||
|
||||
`run-events.ts`:
|
||||
- Add `"stage.retrying"` to the `STAGE_EVENTS` set at `:35`. The projection now stores Retrying state, so the UI must refetch when this event arrives.
|
||||
- Add `queryKeys.runs.billing(runId)` to the `STAGE_EVENTS` invalidation list at `:75`.
|
||||
- Update the `queryKeysForRunEvent` test in `run-events.test.tsx` to verify `stage.retrying` invalidates stages, billing, events, and (when stage_id present) stage turns.
|
||||
|
||||
`run-billing.tsx`:
|
||||
- Detect in-flight via the new `state` field: `state === "running" || state === "retrying"`.
|
||||
- If any row is in-flight, run a `useEffect` `setInterval(..., 1000)` that bumps a `now` state. Render the in-flight row's runtime as `(now − new Date(started_at)) / 1000`.
|
||||
- **Footer total**: while ticking, derive total from the rendered row runtimes — sum up the displayed seconds (which now include the live elapsed for the in-flight row). Otherwise (terminal run) use `billing.totals.runtime_secs` from the server.
|
||||
- Drop the empty-state at `:83` when any in-flight row exists; the table appears as soon as the first stage starts.
|
||||
|
||||
Update `apps/fabro-web/app/routes/run-billing.test.tsx`:
|
||||
- Extend fixtures with `started_at` and `state`.
|
||||
- Add a test for an in-flight row (state = `running`) that asserts (a) the row renders, (b) the footer total includes the elapsed time, (c) the table is shown even when no stage has completed.
|
||||
|
||||
### 8. What stays out of scope
|
||||
|
||||
- **Live tokens during a stage.** Requires a new `agent.turn.completed { usage }` event from `fabro-agent`/`fabro-llm` plus a reducer arm to accumulate onto `StageProjection.usage`. The schema in §1 is ready; instrumenting it is a separate change.
|
||||
- **Per-visit billing rows.** Today's behavior aggregates by node_id (latest visit). One row per retry/revisit is a UX decision separate from this fix.
|
||||
- **Removing `checkpoint.node_outcomes`.** Still used by workflow execution: `artifact.rs:92,134`, `finalize.rs:119,394`, retro/conditionals. Leave it.
|
||||
- **Mixed in-memory/projection reads on `/checkpoint` and `/graph`.** Different shape of issue; not this PR.
|
||||
|
||||
### 9. API round-trip tests
|
||||
|
||||
Files: `lib/crates/fabro-api/tests/stage_projection_round_trip.rs`, `lib/crates/fabro-api/tests/run_billing_stage_round_trip.rs`.
|
||||
|
||||
Extend the representative-JSON cases:
|
||||
|
||||
- `stage_projection_round_trip.rs`: add `started_at`, `duration_ms`, `state` to the JSON fixture and assert they round-trip. Confirms the OpenAPI schema and Rust type stay in lock-step for the new fields.
|
||||
- `run_billing_stage_round_trip.rs`: add `started_at` and `state` to the JSON fixture and assert they round-trip. Add a second case for an in-flight row (`state = "running"`, no `model`, zero `billing`).
|
||||
|
||||
These prevent silent drift if the OpenAPI schema and Rust type ever diverge on the new fields.
|
||||
|
||||
## Files to modify
|
||||
|
||||
- `lib/crates/fabro-types/src/run_projection.rs` — fields + helpers
|
||||
- `lib/crates/fabro-store/src/run_state.rs` — reducer arms (incl. new `StageRetrying`) + tests
|
||||
- `lib/crates/fabro-server/src/server/handler/billing.rs` — both handlers rewritten; delete `active_stage_state_from_events`
|
||||
- `lib/crates/fabro-server/src/server/tests.rs` — keep `list_run_stages_projects_retrying_until_completion`; verify it still passes via the new projection-based path
|
||||
- `lib/crates/fabro-server/src/demo/mod.rs` — fixture updates
|
||||
- `docs/public/api-reference/fabro-api.yaml` — `RunBillingStage`, `RunStage`, `StageProjection`
|
||||
- `lib/packages/fabro-api-client` — regenerated
|
||||
- `apps/fabro-web/app/lib/run-events.ts` — billing invalidation on stage events
|
||||
- `apps/fabro-web/app/routes/run-billing.tsx` — in-flight detection + tick + derived footer total
|
||||
- `apps/fabro-web/app/routes/run-billing.test.tsx` — new fixtures + in-flight + footer-tick assertions
|
||||
- `lib/crates/fabro-api/tests/stage_projection_round_trip.rs` — extend fixture with new fields
|
||||
- `lib/crates/fabro-api/tests/run_billing_stage_round_trip.rs` — extend fixture with new fields, add in-flight case
|
||||
- `apps/fabro-web/app/lib/run-events.test.tsx` — assert `stage.retrying` invalidates billing/stages/events
|
||||
|
||||
## Existing utilities to reuse
|
||||
|
||||
- `RunProjection::iter_stages()` — `lib/crates/fabro-types/src/run_projection.rs:102`
|
||||
- `StageProjection::first_event_seq` — already a `NonZeroU32`, ready as sort key
|
||||
- `StageState` — `lib/crates/fabro-types/src/outcome.rs:111` with `From<StageOutcome>` already wired
|
||||
- `BilledTokenCounts::from_billed_usage` — used by current totals path
|
||||
- `accumulate_model_billing` — `lib/crates/fabro-server/src/server.rs:539`, used for by-model breakdown
|
||||
- chrono pattern: `now.signed_duration_since(...).num_milliseconds().max(0) as f64 / 1000.0` (e.g. `lib/crates/fabro-cli/src/commands/runs/list.rs:99`)
|
||||
|
||||
## Verification
|
||||
|
||||
1. **Reducer unit tests** in `run_state.rs`:
|
||||
- `stage_started_records_started_at_and_running_state`
|
||||
- `stage_completed_records_duration_usage_and_terminal_state`
|
||||
- `stage_failed_records_duration_and_failed_state`
|
||||
- `stage_retrying_sets_retrying_state`
|
||||
- `stage_started_after_retrying_returns_to_running` (transition)
|
||||
2. **Existing test must still pass**: `list_run_stages_projects_retrying_until_completion` (`server/tests.rs:2126`) — covers Retrying via the new projection path.
|
||||
3. **New handler integration tests** in `lib/crates/fabro-server/tests/it/scenario/usage.rs`:
|
||||
- **Mid-run snapshot**: pause workflow with one completed and one in-flight stage; assert `/billing` returns two rows; in-flight row has `state = "running"`, `model = null`, zero `billing` tokens, non-zero `runtime_secs`; totals include the in-flight runtime.
|
||||
- **Retried node, mid-retry**: StageStarted → StageFailed (duration_ms = 10) → StageRetrying → StageStarted (no completion yet); assert the row's `state = "running"` and `runtime_secs` reflects elapsed since the **second** StageStarted, not the failed attempt's 10ms. Pin the regression risk that motivated the `runtime_secs()` priority inversion.
|
||||
- **Retried node, succeeded**: same prefix → StageCompleted; assert one row per node_id (latest visit), state `Succeeded`, duration = final attempt's `duration_ms`.
|
||||
- **Revisited node (loop, multi-node)**: emit A completed → B completed → A revisited+completed (visit=2). Assert (a) two rows total, (b) order is A, B (matches `finalize.rs:113`), (c) A's row carries the latest visit's data (visit=2 duration/usage), not the first visit's. Pins both the dedupe rule and the ordering rule against future drift.
|
||||
4. **Frontend tests** — `run-billing.test.tsx`:
|
||||
- In-flight row renders with runtime > 0.
|
||||
- Footer total ticks while the in-flight row ticks.
|
||||
- Empty-state hidden when an in-flight row exists.
|
||||
5. **End-to-end smoke** — `fabro run repl`, open `/runs/<id>/billing` in dev:
|
||||
- In-flight stage row appears immediately on `stage.started`.
|
||||
- Runtime ticks once per second.
|
||||
- On `stage.completed`, row gets `duration_ms` + tokens; next stage's row appears.
|
||||
- Footer reflects live in-flight runtime.
|
||||
6. **Conformance** — `cargo nextest run -p fabro-server`, `cd apps/fabro-web && bun run typecheck && bun test`, `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings`. Run `cargo insta pending-snapshots` afterwards in case any snapshot tests pick up the new optional fields.
|
||||
|
||||
## Unresolved questions
|
||||
|
||||
- For runs with retried/revisited nodes, is "latest visit per node_id" the right billing display, or should we eventually expose all visits as separate rows? Plan matches current behavior; flagging for future.
|
||||
- `StageProjection.usage` is server-internal (`#[serde(skip)]`) for this PR. If a future consumer of `/runs/{id}/state` needs per-stage tokens, we'd model `BilledModelUsage` as an OpenAPI schema and unskip it — separate change.
|
||||
|
||||
|
||||
## Completed stages
|
||||
- **toolchain**: succeeded
|
||||
- Script: `command -v cargo >/dev/null || { curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y && sudo ln -sf $HOME/.cargo/bin/* /usr/local/bin/; }; cargo --version 2>&1`
|
||||
- Stdout:
|
||||
```
|
||||
cargo 1.95.0 (f2d3ce0bd 2026-03-21)
|
||||
```
|
||||
- Stderr: (empty)
|
||||
- **preflight_compile**: succeeded
|
||||
- Script: `cargo check -q --workspace 2>&1`
|
||||
- Stdout: (empty)
|
||||
- Stderr: (empty)
|
||||
- **preflight_lint**: succeeded
|
||||
- Script: `cargo +nightly-2026-04-14 clippy -q --workspace --all-targets -- -D warnings 2>&1`
|
||||
- Stdout: (empty)
|
||||
- Stderr: (empty)
|
||||
- **implement**: succeeded
|
||||
- Model: claude-opus-4-7, 212.1k tokens in / 72.4k out
|
||||
- Files: /home/daytona/workspace/apps/fabro-web/app/lib/query-keys.test.ts, /home/daytona/workspace/apps/fabro-web/app/lib/run-events.test.tsx, /home/daytona/workspace/apps/fabro-web/app/lib/run-events.ts, /home/daytona/workspace/apps/fabro-web/app/routes/run-billing.test.tsx, /home/daytona/workspace/apps/fabro-web/app/routes/run-billing.tsx, /home/daytona/workspace/docs/public/api-reference/fabro-api.yaml, /home/daytona/workspace/lib/crates/fabro-api/tests/run_billing_stage_round_trip.rs, /home/daytona/workspace/lib/crates/fabro-api/tests/stage_projection_round_trip.rs, /home/daytona/workspace/lib/crates/fabro-server/src/demo/mod.rs, /home/daytona/workspace/lib/crates/fabro-server/src/server/handler/billing.rs, /home/daytona/workspace/lib/crates/fabro-server/src/server/tests.rs, /home/daytona/workspace/lib/crates/fabro-server/tests/it/scenario/usage.rs, /home/daytona/workspace/lib/crates/fabro-store/src/run_state.rs, /home/daytona/workspace/lib/crates/fabro-types/src/run_projection.rs, /home/daytona/workspace/lib/packages/fabro-api-client/src/models/run-billing-stage.ts, /home/daytona/workspace/lib/packages/fabro-api-client/src/models/run-stage.ts, /home/daytona/workspace/lib/packages/fabro-api-client/src/models/stage-projection.ts
|
||||
- **simplify_opus**: succeeded
|
||||
- Model: claude-opus-4-7, 98.5k tokens in / 30.0k out
|
||||
- Files: /home/daytona/workspace/apps/fabro-web/app/components/stage-sidebar.tsx, /home/daytona/workspace/apps/fabro-web/app/lib/stage-sidebar.ts, /home/daytona/workspace/apps/fabro-web/app/lib/time.ts, /home/daytona/workspace/apps/fabro-web/app/routes/run-billing.tsx, /home/daytona/workspace/apps/fabro-web/app/routes/run-stages.tsx, /home/daytona/workspace/lib/crates/fabro-server/src/server/handler/billing.rs, /home/daytona/workspace/lib/crates/fabro-server/tests/it/scenario/usage.rs, /home/daytona/workspace/lib/crates/fabro-store/src/run_state.rs, /home/daytona/workspace/lib/crates/fabro-types/src/run_projection.rs
|
||||
|
||||
|
||||
# Simplify: Code Review and Cleanup
|
||||
|
||||
Review changes vs. origin for reuse, quality, and efficiency. Fix any issues found.
|
||||
|
||||
## Phase 1: Identify Changes
|
||||
|
||||
Run git diff (or git diff HEAD if there are staged changes) to see what changed. If there are no git changes, review the most recently modified files that the user mentioned or that you edited earlier in this conversation.
|
||||
|
||||
## Phase 2: Launch Three Review Agents in Parallel
|
||||
|
||||
Use the Agent tool to launch all three agents concurrently in a single message. Pass each agent the full diff so it has the complete context.
|
||||
|
||||
### Agent 1: Code Reuse Review
|
||||
|
||||
For each change:
|
||||
|
||||
1. Search for existing utilities and helpers that could replace newly written code. Use Grep to find similar patterns elsewhere in the codebase — common locations are utility directories, shared modules, and files adjacent to the changed ones.
|
||||
2. Flag any new function that duplicates existing functionality. Suggest the existing function to use instead.
|
||||
3. Flag any inline logic that could use an existing utility — hand-rolled string manipulation, manual path handling, custom environment checks, ad-hoc type guards, and similar patterns are common candidates.
|
||||
|
||||
Note: This is a greenfield app, so focus on maximizing simplicity and don't worry about changing things to achieve it.
|
||||
|
||||
### Agent 2: Code Quality Review
|
||||
|
||||
Review the same changes for hacky patterns:
|
||||
|
||||
1. Redundant state: state that duplicates existing state, cached values that could be derived, observers/effects that could be direct calls
|
||||
2. Parameter sprawl: adding new parameters to a function instead of generalizing or restructuring existing ones
|
||||
3. Copy-paste with slight variation: near-duplicate code blocks that should be unified with a shared abstraction
|
||||
4. Leaky abstractions: exposing internal details that should be encapsulated, or breaking existing abstraction boundaries
|
||||
5. Stringly-typed code: using raw strings where constants, enums (string unions), or branded types already exist in the codebase
|
||||
|
||||
Note: This is a greenfield app, so be aggressive in optimizing quality.
|
||||
|
||||
### Agent 3: Efficiency Review
|
||||
|
||||
Review the same changes for efficiency:
|
||||
|
||||
1. Unnecessary work: redundant computations, repeated file reads, duplicate network/API calls, N+1 patterns
|
||||
2. Missed concurrency: independent operations run sequentially when they could run in parallel
|
||||
3. Hot-path bloat: new blocking work added to startup or per-request/per-render hot paths
|
||||
4. Unnecessary existence checks: pre-checking file/resource existence before operating (TOCTOU anti-pattern) — operate directly and handle the error
|
||||
5. Memory: unbounded data structures, missing cleanup, event listener leaks
|
||||
6. Overly broad operations: reading entire files when only a portion is needed, loading all items when filtering for one
|
||||
|
||||
## Phase 3: Fix Issues
|
||||
|
||||
Wait for all three agents to complete. Aggregate their findings and fix each issue directly. If a finding is a false positive or not worth addressing, note it and move on — do not argue with the finding, just skip it.
|
||||
|
||||
When done, briefly summarize what was fixed (or confirm the code was already clean).
|
||||
5
stages/007-simplify_gpt@1/provider_used.json
Normal file
5
stages/007-simplify_gpt@1/provider_used.json
Normal file
|
|
@ -0,0 +1,5 @@
|
|||
{
|
||||
"mode": "agent",
|
||||
"provider": "openai",
|
||||
"model": "gpt-5.5"
|
||||
}
|
||||
28
stages/007-simplify_gpt@1/response.md
Normal file
28
stages/007-simplify_gpt@1/response.md
Normal file
|
|
@ -0,0 +1,28 @@
|
|||
Completed the review/cleanup pass. The subagent tool failed twice with an infrastructure “item not found” error before returning findings, so I proceeded with the same reuse/quality/efficiency review locally.
|
||||
|
||||
Fixes applied:
|
||||
|
||||
- Reused existing billing aggregation helper:
|
||||
- `get_run_billing` now uses `BilledTokenCounts::from_billed_usage(...)` for per-stage billing instead of duplicating token-copy logic.
|
||||
- Tightened frontend typing/quality:
|
||||
- Removed unnecessary `StageState` cast/import in `run-billing.tsx`.
|
||||
- Updated billing empty-state copy from “No completed stages yet” to “No stages yet” so it matches the new in-flight behavior.
|
||||
- Ensured touched TS files end with newlines.
|
||||
- Improved live ticking correctness:
|
||||
- `useTickingNow` now refreshes `Date.now()` immediately when ticking becomes active, avoiding a stale first rendered runtime if data arrives after mount.
|
||||
- Strengthened reducer test coverage:
|
||||
- `stage_completed_records_duration_usage_and_terminal_state` now asserts `StageCompleted` stores billing usage, not just duration/state.
|
||||
|
||||
Validation run:
|
||||
|
||||
- `cargo +nightly-2026-04-14 fmt --all`
|
||||
- `cargo nextest run -p fabro-store stage_completed_records_duration_usage_and_terminal_state stage_started_after_retrying_returns_to_running_and_resets_attempt_data`
|
||||
- `cargo nextest run -p fabro-server list_run_stages_projects_retrying_until_completion run_billing_retried_node_then_succeeded_emits_one_row_with_final_attempt_duration run_billing_revisited_node_collapses_to_two_rows_with_latest_visit_data`
|
||||
- `cargo nextest run -p fabro-api stage_projection_round_trips_representative_json run_billing_stage_round_trips_terminal_row_with_started_at_and_state run_billing_stage_round_trips_in_flight_row`
|
||||
- `cargo check -q -p fabro-server -p fabro-store -p fabro-api`
|
||||
- `cargo +nightly-2026-04-14 clippy -q -p fabro-server -p fabro-store --all-targets -- -D warnings`
|
||||
- `cd apps/fabro-web && bun run typecheck`
|
||||
- `cd apps/fabro-web && bun test app/routes/run-billing.test.tsx app/lib/run-events.test.tsx`
|
||||
- `git diff --check`
|
||||
|
||||
All passed.
|
||||
Loading…
Add table
Reference in a new issue