mirror of
https://github.com/fabro-sh/fabro.git
synced 2026-10-06 02:48:25 +00:00
## Summary Improves the web UI's React Doctor audit score by separating reusable helpers from React component modules, tightening effect/state ownership, and extracting real component boundaries in the install wizard, stage activity view, run-files diff browser, RunDetail route, and Runs workspace. The branch removes the previously deferred RunDetail and Runs giant-component diagnostics without changing RunDetail UX, route contracts, action ordering, or Runs workspace behavior. | Metric | Main baseline | Initial PR | Current PR | |--------|---------------|------------|------------| | React Doctor score | 63 | 71 | 99 | | React Doctor errors | 123 | 0 | 0 | | React Doctor warnings | 241 | 163 | 3 | | React Doctor diagnostics | 364 | 163 | 3 | ## Changes - Moves exported helper logic out of component files so Fast Refresh/component-export rules no longer dominate the audit. - Adds a targeted React Doctor config exception for React Router route modules, where non-component exports like route metadata are intentional. - Refactors low-risk state/effect patterns: keyed interview question state, reducer-backed editable run title state, event-owned preview opening, route-keyed insights editor initialization, refresh timer ownership, and selection/derived list cleanup. - Reworks `InstallApp` around an install reducer, a controller hook for install lifecycle state, and focused wizard step components for LLM, server, object-store, sandbox, and GitHub setup. - Moves `RunStages` selected-stage activity into a keyed boundary for panel/debug detail state while preserving stage activity filters across navigation. - Extracts the `RunFiles` loaded diff-browser view from route/query coordination so the route owns data/URL state and the loaded view owns rendering. - Splits `RunDetail` into route-local header, actions, tab shell, docked controls, model, and lifecycle-toast modules; the actions menu now uses grouped descriptors instead of a large boolean/callback prop matrix. - Extracts Runs workspace preference ownership into `useRunsWorkspacePreferences` and moves toolbar rendering into `RunsToolbar`, leaving the route focused on data, DnD state, filtering, and view selection. - Guards `InsightsEditor` query execution with a latest-run id and timeout cleanup so stale or unmounted mock query runs cannot overwrite newer results. - Adds regression coverage for archived-run deletion from RunDetail and stale-result handling in InsightsEditor. - Improves semantic/accessibility coverage with labeled controls, native meter/section semantics, decorative status dots, and clearer unavailable copy. - Removes dead UI code and applies local suppressions only where the rule is a documented false positive or an intentional imperative integration boundary. ## Remaining React Doctor warnings Current score is 99 with 0 errors and 3 warnings. The remaining warnings are intentionally left for separate judgment rather than mechanical churn: - `prefer-useReducer` (3): `AutomationsNew`, `InsightsEditor`, and `CreateSecretForm` need reducers only if they encode real coupled transitions, not simple field setters. ## Verification - `cd apps/fabro-web && bun test app/routes/run-detail.test.ts` -> `22 pass`, `0 fail` - `cd apps/fabro-web && bun test app/routes/insights-editor.test.tsx app/routes/runs.preferences.test.tsx` -> `7 pass`, `0 fail` - `cd apps/fabro-web && bun run typecheck` - `cd apps/fabro-web && bun test --isolate` -> `490 pass`, `0 fail` - `cd apps/fabro-web && bunx react-doctor@latest --full --json > /tmp/fabro-react-doctor-runs-insights.json` -> score `99`, `0` errors, `3` warnings - Earlier branch verification also included `cd apps/fabro-web && bun run build` - `git diff --check` --- [](https://github.com/EveryInc/compound-engineering-plugin) 🤖 Generated with GPT-5 (context not reported, default reasoning) via [Codex](https://openai.com/codex)
261 lines
8.1 KiB
TypeScript
261 lines
8.1 KiB
TypeScript
import { describe, expect, test } from "bun:test";
|
|
import TestRenderer, { act } from "react-test-renderer";
|
|
import { SWRConfig } from "swr";
|
|
import {
|
|
type ApiQuestion,
|
|
QuestionType,
|
|
} from "@qltysh/fabro-api-client";
|
|
|
|
import { InterviewDock } from "./interview-dock";
|
|
import { displayLabel } from "./interview-label";
|
|
import { generatedAxios } from "../lib/api-client";
|
|
|
|
function render(node: React.ReactNode): TestRenderer.ReactTestRenderer {
|
|
let tree: TestRenderer.ReactTestRenderer | undefined;
|
|
act(() => {
|
|
tree = TestRenderer.create(
|
|
<SWRConfig value={{ provider: () => new Map(), dedupingInterval: 0 }}>
|
|
{node}
|
|
</SWRConfig>,
|
|
);
|
|
});
|
|
return tree!;
|
|
}
|
|
|
|
function textContent(node: ReturnType<TestRenderer.ReactTestRenderer["toJSON"]>): string {
|
|
if (!node) return "";
|
|
if (typeof node === "string") return node;
|
|
if (Array.isArray(node)) return node.map(textContent).join("");
|
|
return (node.children ?? []).map(textContent).join("");
|
|
}
|
|
|
|
function instanceText(instance: TestRenderer.ReactTestInstance): string {
|
|
const parts: string[] = [];
|
|
for (const child of instance.children) {
|
|
if (typeof child === "string") parts.push(child);
|
|
else parts.push(instanceText(child));
|
|
}
|
|
return parts.join("");
|
|
}
|
|
|
|
function buttonsByText(
|
|
tree: TestRenderer.ReactTestRenderer,
|
|
): Record<string, TestRenderer.ReactTestInstance> {
|
|
const result: Record<string, TestRenderer.ReactTestInstance> = {};
|
|
for (const button of tree.root.findAllByType("button")) {
|
|
const label = instanceText(button).trim();
|
|
if (label) result[label] = button;
|
|
}
|
|
return result;
|
|
}
|
|
|
|
function makeQuestion(overrides: Partial<ApiQuestion> = {}): ApiQuestion {
|
|
return {
|
|
id: "q-1",
|
|
text: "Approve the deployment plan?",
|
|
stage: "approve_plan",
|
|
question_type: QuestionType.YES_NO,
|
|
options: [],
|
|
allow_freeform: false,
|
|
timeout_seconds: null,
|
|
context_display: null,
|
|
...overrides,
|
|
};
|
|
}
|
|
|
|
describe("InterviewDock", () => {
|
|
test("renders question text and stage in the header", () => {
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[makeQuestion()]} />,
|
|
);
|
|
const text = textContent(tree.toJSON());
|
|
expect(text).toContain("Approve the deployment plan?");
|
|
expect(text).toContain("approve_plan");
|
|
expect(text).toContain("Awaiting input");
|
|
});
|
|
|
|
test("yes/no question shows two buttons", () => {
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[makeQuestion()]} />,
|
|
);
|
|
const buttons = buttonsByText(tree);
|
|
expect(buttons.Yes).toBeDefined();
|
|
expect(buttons.No).toBeDefined();
|
|
});
|
|
|
|
test("yes/no question submits typed yes and no answers", async () => {
|
|
const submitted: unknown[] = [];
|
|
const originalAdapter = generatedAxios.defaults.adapter;
|
|
generatedAxios.defaults.adapter = async (config) => {
|
|
submitted.push(JSON.parse(String(config.data)));
|
|
return {
|
|
data: undefined,
|
|
status: 204,
|
|
statusText: "No Content",
|
|
headers: {},
|
|
config,
|
|
};
|
|
};
|
|
|
|
try {
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[makeQuestion()]} />,
|
|
);
|
|
const buttons = buttonsByText(tree);
|
|
|
|
await act(async () => {
|
|
buttons.Yes.props.onClick();
|
|
await Promise.resolve();
|
|
});
|
|
await act(async () => {
|
|
buttons.No.props.onClick();
|
|
await Promise.resolve();
|
|
});
|
|
|
|
expect(submitted).toEqual([{ kind: "yes" }, { kind: "no" }]);
|
|
} finally {
|
|
generatedAxios.defaults.adapter = originalAdapter;
|
|
}
|
|
});
|
|
|
|
test("multiple choice question renders option buttons with stripped accelerator prefixes", () => {
|
|
const question = makeQuestion({
|
|
question_type: QuestionType.MULTIPLE_CHOICE,
|
|
options: [
|
|
{ key: "A", label: "[A] Approve" },
|
|
{ key: "R", label: "[R] Revise" },
|
|
],
|
|
});
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[question]} />,
|
|
);
|
|
const buttons = buttonsByText(tree);
|
|
expect(buttons.Approve).toBeDefined();
|
|
expect(buttons.Revise).toBeDefined();
|
|
});
|
|
|
|
test("multiple choice renders option descriptions as display text", () => {
|
|
const question = makeQuestion({
|
|
question_type: QuestionType.MULTIPLE_CHOICE,
|
|
options: [
|
|
{
|
|
key: "A",
|
|
label: "[A] Approve",
|
|
description: "Deploy the current patch",
|
|
preview: "<b>not rendered specially</b>",
|
|
},
|
|
],
|
|
});
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[question]} />,
|
|
);
|
|
const text = textContent(tree.toJSON());
|
|
expect(text).toContain("Approve");
|
|
expect(text).toContain("Deploy the current patch");
|
|
expect(text).not.toContain("<b>not rendered specially</b>");
|
|
});
|
|
|
|
test("freeform question renders a textarea and disables send when empty", () => {
|
|
const question = makeQuestion({
|
|
question_type: QuestionType.FREEFORM,
|
|
});
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[question]} />,
|
|
);
|
|
const textareas = tree.root.findAllByType("textarea");
|
|
expect(textareas).toHaveLength(1);
|
|
const sendButton = tree.root.findByProps({ type: "submit" });
|
|
expect(sendButton.props.disabled).toBe(true);
|
|
});
|
|
|
|
test("multi-select shows submit button disabled until at least one option is selected", () => {
|
|
const question = makeQuestion({
|
|
question_type: QuestionType.MULTI_SELECT,
|
|
options: [
|
|
{ key: "a", label: "[A] Apples" },
|
|
{ key: "b", label: "[B] Bananas" },
|
|
],
|
|
});
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[question]} />,
|
|
);
|
|
const buttons = buttonsByText(tree);
|
|
const submit = buttons["Submit selection"];
|
|
expect(submit).toBeDefined();
|
|
expect(submit.props.disabled).toBe(true);
|
|
|
|
act(() => {
|
|
buttons.Apples.props.onClick();
|
|
});
|
|
const submitAfter = buttonsByText(tree)["Submit selection"];
|
|
expect(submitAfter.props.disabled).toBe(false);
|
|
});
|
|
|
|
test("multiple choice with allow_freeform renders both buttons and a textarea", () => {
|
|
const question = makeQuestion({
|
|
question_type: QuestionType.MULTIPLE_CHOICE,
|
|
allow_freeform: true,
|
|
options: [{ key: "A", label: "[A] Approve" }],
|
|
});
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[question]} />,
|
|
);
|
|
expect(buttonsByText(tree).Approve).toBeDefined();
|
|
expect(tree.root.findAllByType("textarea")).toHaveLength(1);
|
|
});
|
|
|
|
test("shows '+N more pending' pill when multiple questions are queued", () => {
|
|
const tree = render(
|
|
<InterviewDock
|
|
runId="run-1"
|
|
questions={[
|
|
makeQuestion({ id: "q-1", stage: "stage-a" }),
|
|
makeQuestion({ id: "q-2", stage: "stage-b" }),
|
|
makeQuestion({ id: "q-3", stage: "stage-c" }),
|
|
]}
|
|
/>,
|
|
);
|
|
const text = textContent(tree.toJSON());
|
|
expect(text).toContain("2");
|
|
expect(text).toContain("more pending");
|
|
});
|
|
|
|
test("renders nothing when questions list is empty", () => {
|
|
const tree = render(<InterviewDock runId="run-1" questions={[]} />);
|
|
expect(tree.toJSON()).toBeNull();
|
|
});
|
|
|
|
test("renders the optional context_display section", () => {
|
|
const question = makeQuestion({
|
|
context_display: "Plan:\n1. Deploy\n2. Verify",
|
|
});
|
|
const tree = render(
|
|
<InterviewDock runId="run-1" questions={[question]} />,
|
|
);
|
|
const text = textContent(tree.toJSON());
|
|
expect(text).toContain("Context from preceding stage");
|
|
expect(text).toContain("1. Deploy");
|
|
});
|
|
});
|
|
|
|
describe("displayLabel", () => {
|
|
test("strips bracketed accelerator", () => {
|
|
expect(displayLabel("[A] Approve")).toBe("Approve");
|
|
});
|
|
|
|
test("strips parenthesis accelerator", () => {
|
|
expect(displayLabel("Y) Yes, deploy")).toBe("Yes, deploy");
|
|
});
|
|
|
|
test("strips dash accelerator", () => {
|
|
expect(displayLabel("Y - Yes, deploy")).toBe("Yes, deploy");
|
|
});
|
|
|
|
test("returns original label when no accelerator pattern matches", () => {
|
|
expect(displayLabel("Plain label")).toBe("Plain label");
|
|
});
|
|
|
|
test("falls back to original label when stripping yields empty string", () => {
|
|
expect(displayLabel("[A]")).toBe("[A]");
|
|
});
|
|
});
|