Repository navigation
fix(webapp): forward condition value and windowSec in queryWatchFromPatch (#5065) - #5079
MilindChatrabhuji wants to merge 1 commit into
Conversation
|
|
Hi @MilindChatrabhuji, thanks for your interest in contributing! This project requires that pull request authors are vouched, and you are not in the list of vouched users. This PR will be closed automatically. See https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md for more details. |
|
Caution Review failedThe pull request is closed. Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| const prefillWindowMinutes = | ||
| prefill.windowSec !== undefined ? Math.round(prefill.windowSec / 60) : undefined; |
There was a problem hiding this comment.
🟡 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.
| const prefillWindowMinutes = | |
| prefill.windowSec !== undefined ? Math.round(prefill.windowSec / 60) : undefined; | |
| const prefillWindowMinutes = | |
| prefill.windowSec !== undefined ? Math.ceil(prefill.windowSec / 60) : undefined; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| value: patch.condition?.value, | ||
| windowSec: patch.condition?.windowSec, |
There was a problem hiding this comment.
🟡 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.
Was this helpful? React with 👍 or 👎 to provide feedback.
| 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, | ||
| }); |
There was a problem hiding this comment.
🔍 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.
Was this helpful? React with 👍 or 👎 to provide feedback.
Description
Fixes #5065
When an agent suggests a query alert draft patch,
queryWatchFromPatchdroppedcondition.valueandcondition.windowSec, only forwardingoperator. This resulted inuseQueryAlertDraftinitializingvalueTextto"", leavingdraft.valueValidasfalseand preventing threshold step pre-selection inQueryAlertTranscript.Changes
queryWatchFromPatch: Forwardvalue: patch.condition?.valueandwindowSec: patch.condition?.windowSectoQueryWatchPrefill.QueryWatchPrefill&QueryAlertPrefill: Added optionalvalue?: numberandwindowSec?: numberfields.QueryAlertTranscript: PassvalueandwindowSecthrough toQueryAlertPrefill.useQueryAlertDraft: InitializevalueTextfromprefill.value(if defined), and initializewindowTextfromprefill.windowSec(converted to minutes and bounded within limits) when provided.draftPatch.test.tsverifying thatcondition.valueandwindowSecare accurately preserved inqueryWatchFromPatch.