Skip to content

fix(webapp): forward condition value and windowSec in queryWatchFromPatch (#5065) #5079

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
MilindChatrabhuji wants to merge 1 commit into from
Closed

fix(webapp): forward condition value and windowSec in queryWatchFromPatch (#5065) #5079

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
4 changes: 4 additions & 0 deletions apps/webapp/app/components/alerts/QueryAlertTranscript.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ export type QueryWatchPrefill = {
mode?: QueryAlertMode;
measure?: string;
operator?: ">" | "<";
value?: number;
windowSec?: number;
};

export function queryWatchPrefill(
Expand Down Expand Up @@ -105,6 +107,8 @@ export function QueryAlertTranscript({
mode: prefill.mode,
measure: prefill.measure,
operator: prefill.operator,
value: prefill.value,
windowSec: prefill.windowSec,
...analyzed,
}
: undefined;
Expand Down
2 changes: 2 additions & 0 deletions apps/webapp/app/components/alerts/queryAlertLink.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export type QueryAlertPrefill = {
mode?: QueryAlertMode;
measure?: string;
operator?: ">" | "<";
value?: number;
windowSec?: number;
};

export function queryAlertNameKey(mode: QueryAlertMode, measure?: string) {
Expand Down
14 changes: 12 additions & 2 deletions apps/webapp/app/components/alerts/useQueryAlertDraft.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,13 +56,23 @@ export function useQueryAlertDraft(
const measure = mode === "match" ? undefined : signal.measure;

const [operator, setOperator] = useState<">" | "<">(prefill.operator ?? ">");
const [valueText, setValueText] = useState("");
const [valueText, setValueText] = useState(
prefill.value !== undefined ? String(prefill.value) : ""
);
const value = valueText.trim() === "" ? undefined : Number(valueText);
const valueValid = mode !== "threshold" || (value !== undefined && Number.isFinite(value));

const bounds = queryWindowBounds(analysis.limits);
const defaultWindow = defaultQueryWindowMinutes(prefill.period, analysis.limits);
const [windowText, setWindowText] = useState(() => String(defaultWindow));
const prefillWindowMinutes =
prefill.windowSec !== undefined ? Math.round(prefill.windowSec / 60) : undefined;
Comment on lines +67 to +68

@devin-ai-integration devin-ai-integration Bot Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Suggested alert window gets shortened

For a 601-second query window, prefillWindowMinutes selects 10 minutes instead of 11. The resulting alert watches less history than requested.

Learn more

Alert windows are entered and saved as whole minutes. The agent supplies condition.windowSec in seconds, which is forwarded to this hook. Rounding to the nearest minute can produce a shorter window, although applyDraftPatch rounds the same input upward for curated alerts. The alert then evaluates a different window than the one the agent suggested.

Example: With a minimum of 10 minutes, a 601-second suggestion becomes 10 minutes here, whereas rounding up gives 11 minutes.

Recommended fix: Use Math.ceil when converting seconds to minutes, as applyDraftPatch does; keep the existing bounds check.

Suggested change
const prefillWindowMinutes =
prefill.windowSec !== undefined ? Math.round(prefill.windowSec / 60) : undefined;
const prefillWindowMinutes =
prefill.windowSec !== undefined ? Math.ceil(prefill.windowSec / 60) : undefined;

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

const initialWindow =
prefillWindowMinutes !== undefined &&
prefillWindowMinutes >= bounds.min &&
prefillWindowMinutes <= bounds.max
? prefillWindowMinutes
: defaultWindow;
const [windowText, setWindowText] = useState(() => String(initialWindow));
const windowMinutes = mode === "match" ? defaultWindow : Number(windowText);
const windowValid =
mode === "match" ||
Expand Down
18 changes: 18 additions & 0 deletions apps/webapp/app/components/dashboard-agent/alert-creation/draftPatch.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
alertDraftPatchSchema,
applyDraftPatch,
errorGroupToResolve,
queryWatchFromPatch,
type AlertDraftPatch,
} from "./draftPatch";
import { draftResultLines } from "./useCreationAgentSend";
Expand Down Expand Up @@ -265,6 +266,23 @@ describe("below-threshold operator", () => {
const patch = { area: "query", query: "SELECT 1", condition: { operator: "<" } };
expect(alertDraftPatchSchema.safeParse(patch).success).toBe(true);
});

it("forwards condition value and windowSec in queryWatchFromPatch", () => {
const patch = {
area: "query",
query: "SELECT count() FROM runs",
querySignalChoice: { mode: "threshold" as const, measure: "count" },
condition: { operator: ">" as const, value: 42, windowSec: 1800 },
};
expect(queryWatchFromPatch(patch)).toEqual({
query: "SELECT count() FROM runs",
mode: "threshold",
measure: "count",
operator: ">",
value: 42,
windowSec: 1800,
});
Comment on lines +270 to +284

@devin-ai-integration devin-ai-integration Bot Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Prefill test does not cover creation

The new test checks forwarding but not the displayed threshold or saved window. A flow test can cover both values through alert creation.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

});
});

describe("unknown-key rejection", () => {
Expand Down
2 changes: 2 additions & 0 deletions apps/webapp/app/components/dashboard-agent/alert-creation/draftPatch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,8 @@ export function queryWatchFromPatch(patch: AlertDraftPatch): QueryWatchPrefill |
mode: patch.querySignalChoice?.mode,
measure: patch.querySignalChoice?.measure,
operator: patch.condition?.operator,
value: patch.condition?.value,
windowSec: patch.condition?.windowSec,
Comment on lines +70 to +71

@devin-ai-integration devin-ai-integration Bot Oct 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Alert fix missing release note

This user-facing webapp fix has no .server-changes entry. The alert prefill improvement will be absent from customer release notes.

Learn more

The agent now passes suggested alert thresholds and windows into the query alert setup. That changes behavior users can notice. The repository requires a .server-changes/ entry for a user-facing change confined to server apps; this PR changes only the webapp and has no entry. Without one, the release process has no note to include for this fix.

Example: A user creating an alert from an agent suggestion now sees the suggested threshold and window populated, but the release notes omit the fix.

Recommended fix: Add a .server-changes/ file with area: webapp, type: fix, and a one-line description of the user-visible prefill improvement.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

};
}

Expand Down