Skip to content

Commit 07759d0

Browse files
camielvsclaude
andcommitted
address pr feedback
- Render OnboardingHero once, drive placement via CSS order toggle - Document why derived onboarding steps don't emit step.completed - Reconcile onboarding progress cache on PATCH failure via onError Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 747a387 commit 07759d0

9 files changed

Lines changed: 96 additions & 13 deletions

File tree

src/components/Learn/OnboardingHero.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { Button } from "@/components/ui/button";
33
import { Icon } from "@/components/ui/icon";
44
import { BlockStack, InlineStack } from "@/components/ui/layout";
55
import { Heading, Paragraph } from "@/components/ui/typography";
6+
import { cn } from "@/lib/utils";
67
import { useOnboarding } from "@/providers/OnboardingProvider/OnboardingProvider";
78

89
function scrollNearestScrollableToTop(el: HTMLElement | null) {
@@ -21,12 +22,15 @@ function scrollNearestScrollableToTop(el: HTMLElement | null) {
2122
window.scrollTo({ top: 0, behavior: "smooth" });
2223
}
2324

24-
export function OnboardingHero() {
25+
export function OnboardingHero({ className }: { className?: string }) {
2526
const { isComplete, dismissed, dismiss, reopen } = useOnboarding();
2627

2728
if (dismissed) {
2829
return (
29-
<InlineStack align="end" className="text-muted-foreground opacity-50">
30+
<InlineStack
31+
align="end"
32+
className={cn("text-muted-foreground opacity-50", className)}
33+
>
3034
<Button
3135
variant="ghost"
3236
size="sm"
@@ -46,7 +50,12 @@ export function OnboardingHero() {
4650
}
4751

4852
return (
49-
<div className="relative rounded-xl border border-border bg-linear-to-br from-primary/5 to-transparent p-6">
53+
<div
54+
className={cn(
55+
"relative rounded-xl border border-border bg-linear-to-br from-primary/5 to-transparent p-6",
56+
className,
57+
)}
58+
>
5059
<Button
5160
variant="ghost"
5261
size="icon"

src/components/shared/Submitters/Tangle/TangleSubmitter.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import useCooldownTimer from "@/hooks/useCooldownTimer";
1212
import useToastNotification from "@/hooks/useToastNotification";
1313
import { cn } from "@/lib/utils";
1414
import { useBackend } from "@/providers/BackendProvider";
15+
import { ONBOARDING_MY_RUN_COUNT_KEY } from "@/providers/OnboardingProvider/onboardingQueryKeys";
1516
import { useTourMockBackend } from "@/providers/TourProvider/tourMockBackend";
1617
import { APP_ROUTES } from "@/routes/router";
1718
import { updateRunAnnotation } from "@/services/pipelineRunService";
@@ -91,6 +92,11 @@ function useSubmitPipeline() {
9192
await queryClient.invalidateQueries({
9293
queryKey: ["pipelineRuns"],
9394
});
95+
// Refresh the onboarding checklist's run-count so a first run flips
96+
// `execute_run` immediately rather than after the 5-minute stale window.
97+
await queryClient.invalidateQueries({
98+
queryKey: ONBOARDING_MY_RUN_COUNT_KEY,
99+
});
94100
},
95101
});
96102
}

src/providers/OnboardingProvider/OnboardingProvider.test.tsx

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { beforeEach, describe, expect, it, vi } from "vitest";
66
import { emitUserPipelineWritten } from "@/utils/userPipelineWriteEvents";
77

88
import { OnboardingProvider, useOnboarding } from "./OnboardingProvider";
9+
import { ONBOARDING_MY_RUN_COUNT_KEY } from "./onboardingQueryKeys";
910

1011
const fetchWithErrorHandling = vi.hoisted(() =>
1112
vi.fn<(url: string, options?: RequestInit) => Promise<unknown>>(),
@@ -49,6 +50,21 @@ function render() {
4950
return renderHook(() => useOnboarding(), { wrapper });
5051
}
5152

53+
function renderWithClient() {
54+
const queryClient = new QueryClient({
55+
defaultOptions: { queries: { retry: false } },
56+
});
57+
const clientWrapper = ({ children }: { children: ReactNode }) => (
58+
<QueryClientProvider client={queryClient}>
59+
<OnboardingProvider>{children}</OnboardingProvider>
60+
</QueryClientProvider>
61+
);
62+
return {
63+
...renderHook(() => useOnboarding(), { wrapper: clientWrapper }),
64+
queryClient,
65+
};
66+
}
67+
5268
function completedSteps(result: { current: ReturnType<typeof useOnboarding> }) {
5369
return result.current.steps
5470
.filter((step) => step.completed)
@@ -110,6 +126,37 @@ describe("OnboardingProvider", () => {
110126
expect(patched()).toBe(false);
111127
});
112128

129+
it("refreshes execute_run when the run-count query is invalidated after a run is created", async () => {
130+
const { result, queryClient } = renderWithClient();
131+
132+
await waitFor(() => expect(result.current.total).toBe(4));
133+
expect(completedSteps(result)).not.toContain("execute_run");
134+
135+
// A run now exists, and the run-create mutation path invalidates the
136+
// shared onboarding run-count key — execute_run should flip without
137+
// waiting out the query's stale window.
138+
runsPayload = { pipeline_runs: [{ id: "run-1" }] };
139+
await act(async () => {
140+
await queryClient.invalidateQueries({
141+
queryKey: ONBOARDING_MY_RUN_COUNT_KEY,
142+
});
143+
});
144+
145+
await waitFor(() =>
146+
expect(completedSteps(result)).toContain("execute_run"),
147+
);
148+
expect(patched()).toBe(false);
149+
});
150+
151+
it("treats a malformed run-count payload as zero runs", async () => {
152+
runsPayload = { pipeline_runs: "not-an-array" };
153+
154+
const { result } = render();
155+
156+
await waitFor(() => expect(result.current.total).toBe(4));
157+
expect(completedSteps(result)).not.toContain("execute_run");
158+
});
159+
113160
it("persists create_pipeline when the user writes a pipeline", async () => {
114161
const { result } = render();
115162

src/providers/OnboardingProvider/OnboardingProvider.tsx

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { useQuery } from "@tanstack/react-query";
22
import { type ReactNode, useEffect, useState } from "react";
33

4-
import type { ListPipelineJobsResponse } from "@/api/types.gen";
54
import { useDocsVisitTracking } from "@/hooks/useDocsVisitTracking";
65
import {
76
createRequiredContext,
@@ -15,13 +14,15 @@ import {
1514
filtersToFilterQuery,
1615
parseFilterParam,
1716
} from "@/utils/pipelineRunFilterUtils";
17+
import { isRecord } from "@/utils/typeGuards";
1818
import { subscribeUserPipelineWritten } from "@/utils/userPipelineWriteEvents";
1919

2020
import {
2121
type OnboardingSteps,
2222
useOnboardingProgress,
2323
usePersistOnboardingProgress,
2424
} from "./onboardingProgress";
25+
import { ONBOARDING_MY_RUN_COUNT_KEY } from "./onboardingQueryKeys";
2526
import {
2627
ONBOARDING_STEP_IDS,
2728
ONBOARDING_STEPS,
@@ -31,6 +32,11 @@ import {
3132
const PIPELINE_RUNS_QUERY_URL = "/api/pipeline_runs/";
3233
const STALE_MS = 1000 * 60 * 5;
3334

35+
function countPipelineRuns(payload: unknown): number {
36+
if (!isRecord(payload) || !Array.isArray(payload.pipeline_runs)) return 0;
37+
return payload.pipeline_runs.length;
38+
}
39+
3440
export interface OnboardingStep extends OnboardingStepMeta {
3541
completed: boolean;
3642
}
@@ -54,17 +60,15 @@ function useHasMyRun(): boolean {
5460
const filterQuery = filtersToFilterQuery(parseFilterParam("created_by:me"));
5561

5662
const { data } = useQuery({
57-
queryKey: ["onboarding", "myRunCount", backendUrl],
63+
queryKey: [...ONBOARDING_MY_RUN_COUNT_KEY, backendUrl],
5864
enabled: available && Boolean(backendUrl),
5965
staleTime: STALE_MS,
6066
refetchOnWindowFocus: false,
6167
queryFn: async () => {
6268
const url = new URL(PIPELINE_RUNS_QUERY_URL, backendUrl);
6369
if (filterQuery) url.searchParams.set("filter_query", filterQuery);
64-
const payload = (await fetchWithErrorHandling(
65-
url.toString(),
66-
)) as ListPipelineJobsResponse;
67-
return payload.pipeline_runs?.length ?? 0;
70+
const payload = await fetchWithErrorHandling(url.toString());
71+
return countPipelineRuns(payload);
6872
},
6973
});
7074
return (data ?? 0) > 0;

src/providers/OnboardingProvider/onboardingProgress.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,12 @@ export function usePersistOnboardingProgress() {
9898
method: "PATCH",
9999
headers: { "Content-Type": "application/json" },
100100
body: JSON.stringify({ settings: { [ONBOARDING_KEY]: next } }),
101-
// Often fired just before a navigation/reload; survive page unload.
102101
keepalive: true,
103102
});
104103
},
104+
onError: () => {
105+
queryClient.invalidateQueries({ queryKey: queryKey(backendUrl) });
106+
},
105107
});
106108

107109
return (next: OnboardingProgress) => {
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Shared so the run-create mutation paths can invalidate the same query the
2+
// onboarding checklist reads to derive its `execute_run` step. Kept dependency
3+
// free so submitters can import it without pulling in the provider tree.
4+
export const ONBOARDING_MY_RUN_COUNT_KEY = [
5+
"onboarding",
6+
"myRunCount",
7+
] as const;

src/routes/Dashboard/Learn/LearnHomeView.tsx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export function LearnHomeView() {
2626
</BlockStack>
2727
</BlockStack>
2828

29-
{!dismissed && <OnboardingHero />}
29+
<OnboardingHero className={dismissed ? "order-last" : undefined} />
3030

3131
<div className="grid grid-cols-1 lg:grid-cols-2 gap-4">
3232
<TipOfTheDay />
@@ -43,8 +43,6 @@ export function LearnHomeView() {
4343
<HelpCard />
4444
</div>
4545
</div>
46-
47-
{dismissed && <OnboardingHero />}
4846
</BlockStack>
4947
);
5048
}

src/routes/v2/pages/Editor/components/AiChat/toolBridge.test.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
Output,
99
Task,
1010
} from "@/models/componentSpec";
11+
import { ONBOARDING_MY_RUN_COUNT_KEY } from "@/providers/OnboardingProvider/onboardingQueryKeys";
1112
import type { UndoGroupable } from "@/routes/v2/shared/nodes/types";
1213

1314
vi.mock("@/services/componentService", () => ({
@@ -444,6 +445,9 @@ describe("createEditorToolBridge", () => {
444445
expect(urlArg).toBe(TEST_BACKEND_URL);
445446
expect(optionsArg.authorizationToken).toBe("auth-token");
446447
expect(invalidate).toHaveBeenCalledWith({ queryKey: ["pipelineRuns"] });
448+
expect(invalidate).toHaveBeenCalledWith({
449+
queryKey: ONBOARDING_MY_RUN_COUNT_KEY,
450+
});
447451
});
448452

449453
it("returns submission failure with the helper's error message", async () => {

src/routes/v2/shared/components/AiChat/toolBridge/runBridge.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import {
2626
truncateExecutionDetails,
2727
} from "@/agent/util/truncate";
2828
import { serializeComponentSpec } from "@/models/componentSpec/serialization/serialize";
29+
import { ONBOARDING_MY_RUN_COUNT_KEY } from "@/providers/OnboardingProvider/onboardingQueryKeys";
2930
import {
3031
fetchContainerExecutionState,
3132
fetchContainerLog,
@@ -85,6 +86,11 @@ export function createRunBridgeHandlers(deps: BridgeDeps): RunHandlers {
8586
}
8687
// Refresh both the editor list (per pipeline) and the home runs page.
8788
deps.queryClient?.invalidateQueries({ queryKey: ["pipelineRuns"] });
89+
// Keep the onboarding checklist's run-count fresh so a first run flips
90+
// `execute_run` immediately rather than after its stale window.
91+
deps.queryClient?.invalidateQueries({
92+
queryKey: ONBOARDING_MY_RUN_COUNT_KEY,
93+
});
8894
return {
8995
success: true,
9096
runId: String(submission.run.id),

0 commit comments

Comments
 (0)