Skip to content

[WRONG BRANCH] fix(gui): re-enable invalidated routing dry-runs #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
luvs01 wants to merge 1 commit into from
Closed

[WRONG BRANCH] fix(gui): re-enable invalidated routing dry-runs #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions gui/src/pages/RoutingProfiles.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export default function RoutingProfiles({ apiBase }: { apiBase: string }) {

const clearDryRun = useCallback(() => {
dryRunGenerationRef.current += 1;
setRunning(false);
setDryRunResult(null);
setDryRunError("");
}, []);
Comment on lines 84 to 89

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Performance & Scalability | 🔵 Trivial

Confirm whether clearDryRun() must cancel the network request.

clearDryRun() resets running and invalidates the generation, but runDryRun() leaves the existing fetch() in flight. If the user starts another dry-run, the invalidated POST can run in parallel with the new POST. The generation guard prevents stale UI results, but it does not stop server-side work. If the management API performs non-trivial work, add an AbortController for each run and abort it from clearDryRun(); otherwise document that invalidation cancels only the UI result.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gui/src/pages/RoutingProfiles.tsx` around lines 84 - 89, Update clearDryRun
and runDryRun to track each fetch with an AbortController, aborting the active
request when clearDryRun invalidates the run; preserve the generation guard and
ensure abort errors do not surface as ordinary dry-run failures.

Expand Down
51 changes: 50 additions & 1 deletion gui/tests/routing-profiles.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,56 @@ test("routing clears stale dry-run results when switching profiles", async () =>
}
});

test("routing re-enables dry-run after inputs invalidate a pending request", async () => {
let releaseDryRun!: () => void;
const dryRunGate = new Promise<void>(resolve => {
releaseDryRun = resolve;
});

installFetch(async (url, init) => {
if (url.endsWith("/api/routing-profiles") && (init?.method ?? "GET") === "GET") {
return Response.json({ profiles: [PROFILE] });
}
if (url.endsWith("/api/routing-analytics")) {
return Response.json(ANALYTICS);
}
if (url.endsWith("/api/routing-profiles/dry-run") && init?.method === "POST") {
await dryRunGate;
return Response.json(DRY_RUN_OK);
}
return new Response("missing", { status: 404 });
});

const { container, root } = await mountPage();
try {
const evaluate = [...container.querySelectorAll<HTMLButtonElement>("button")]
.find(button => button.textContent?.includes("Evaluate candidates"));
Comment on lines +275 to +276

@coderabbitai coderabbitai Bot Aug 7, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use the locale value or a stable selector for the evaluation button.

The test searches for the translated label "Evaluate candidates". A locale change can make this test fail before it checks dry-run behavior. Use the routing.dryRunRun locale value or add a stable selector to the button instead of duplicating user-visible text.

As per path instructions, user-visible strings in gui/** must go through the i18n locale files rather than hardcoded text.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@gui/tests/routing-profiles.test.tsx` around lines 275 - 276, Update the
evaluation-button lookup in the routing profile test to avoid hardcoded
user-visible text: use the existing i18n locale value for routing.dryRunRun or
add and query a stable selector on the button. Ensure the test remains resilient
to locale changes and follows the gui/** localization requirement.

Source: Path instructions

const tools = container.querySelector<HTMLInputElement>('input[type="checkbox"]');
expect(evaluate).toBeTruthy();
expect(tools).toBeTruthy();

await act(async () => { evaluate!.click(); });
expect(evaluate!.disabled).toBe(true);

await act(async () => { tools!.click(); });
expect(evaluate!.disabled).toBe(false);

await act(async () => {
releaseDryRun();
await Promise.resolve();
});
await tick(3);

expect(evaluate!.disabled).toBe(false);
expect(container.textContent).not.toContain("0.910");
} finally {
await act(async () => {
releaseDryRun();
root.unmount();
});
}
});

test("routing refreshes the selected profile after reload", async () => {
let profilesPayload: unknown[] = [PROFILE];
installFetch((url, init) => {
Expand Down Expand Up @@ -344,4 +394,3 @@ test("routing ignores a stale load body that finishes after a newer retry", asyn
});
}
});

Loading