fix(frontend): scope agent triggers to the revision they were added on - #5182
fix(frontend): scope agent triggers to the revision they were added on#5182ashrafchowdury wants to merge 2 commits into
Conversation
…xt matching logic
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds trigger-origin metadata (revision id, variant id, revision version) tracked via new ChangesTrigger origin scoping and default metadata
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TriggerManagementSection
participant triggerScope
participant SubscriptionOrScheduleDrawer
participant BackendAPI
User->>TriggerManagementSection: Add trigger
TriggerManagementSection->>triggerScope: getAgentTriggerContext(entityId, workflow)
triggerScope-->>TriggerManagementSection: AgentTriggerContext
TriggerManagementSection->>triggerScope: buildAgentTriggerOrigin(context)
triggerScope-->>TriggerManagementSection: AgentTriggerOrigin
TriggerManagementSection->>triggerScope: addAgentTriggerOriginMeta(null, origin)
triggerScope-->>TriggerManagementSection: defaultMeta
TriggerManagementSection->>SubscriptionOrScheduleDrawer: openDrawer(defaultMeta, ...)
User->>SubscriptionOrScheduleDrawer: Submit create form
SubscriptionOrScheduleDrawer->>BackendAPI: create request with meta=defaultMeta
BackendAPI-->>SubscriptionOrScheduleDrawer: created trigger
Note over TriggerManagementSection,triggerScope: Later, listing triggers
TriggerManagementSection->>triggerScope: agentTriggerMatchesContext(trigger, context, agentIds)
triggerScope-->>TriggerManagementSection: boolean match result
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Tools execution failed with the following error: Failed to run tools: 13 INTERNAL: Received RST_STREAM with code 2 (Internal server error) 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
dosubot
Bot
added
size:L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
web/packages/agenta-entity-ui/src/DrillInView/SchemaControls/TriggerManagementSection.tsx (1)
878-913: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winInclude
defaultMetain the add-menu memo dependencies.Line 899 and Line 913 now close over
defaultMeta, but thegroupsuseMemodependency list omits it. If revision/version data resolves after the first render, the drawers can receive stale origin metadata.Proposed fix
[ openSubscriptionDrawer, openScheduleDrawer, defaultReferences, defaultBoundLabel, + defaultMeta, entityId, ],
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 478e45cd-4cfb-4dea-8c87-a022f206ba3e
📒 Files selected for processing (6)
web/packages/agenta-entities/src/gatewayTrigger/state/atoms.tsweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/TriggerManagementSection.tsxweb/packages/agenta-entity-ui/src/DrillInView/SchemaControls/triggerScope.tsweb/packages/agenta-entity-ui/src/gatewayTrigger/drawers/TriggerScheduleDrawer.tsxweb/packages/agenta-entity-ui/src/gatewayTrigger/drawers/TriggerSubscriptionDrawer.tsxweb/packages/agenta-entity-ui/tests/unit/triggerScope.test.ts
| function finiteNumber(value: unknown): number | null { | ||
| const number = typeof value === "number" ? value : Number(value) | ||
| return Number.isFinite(number) ? number : null | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve null revision versions instead of coercing them to 0.
Number(null) returns 0, so missing/null revision_version is treated as v0 and can make origin-scoped triggers match later revisions when version metadata is unavailable.
Proposed fix
function finiteNumber(value: unknown): number | null {
+ if (value == null || value === "") return null
const number = typeof value === "number" ? value : Number(value)
return Number.isFinite(number) ? number : null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function finiteNumber(value: unknown): number | null { | |
| const number = typeof value === "number" ? value : Number(value) | |
| return Number.isFinite(number) ? number : null | |
| } | |
| function finiteNumber(value: unknown): number | null { | |
| if (value == null || value === "") return null | |
| const number = typeof value === "number" ? value : Number(value) | |
| return Number.isFinite(number) ? number : null | |
| } |
There was a problem hiding this comment.
hmmm.. not sure here @ashrafchowdury
It feels to me that it is a better UX to have the trigger show in all revisions, but maybe instead clarify in the card in the playground which revision it triggers to.
Because in many cases the trigger was set to a wrong/old revision and I wanted to update it, not create a new one. Assuming I will have many versions, and I make a mistake, now I need to search old revisions looking for my trigger to change it.
Also, say I make a mistake and I add a trigger to a previous pinned veriosn, now I create a new one, and I don't see any trigger, I might assume I don't have a trigger and add a new one. Now I have two triggers.
Maybe you can share which user flow / use case did you stumble upon that brought you to this solution
|
For me, it was not normal to see the trigger I had added in the updated variant/revision and suddenly see the trigger on the previous default revision-1/variants |
|
@mmabrouk any thoughts on this, otherwise lets close it |
|
Thank you @ashrafchowdury for the video. It explains the issue very well. I think the solution is to 1. show the trigger 2. show in the playground card which revision the trigger triggers with a clear warning that it triggers a different version in case that is true. I will close this and create an issue based on this linking to this one |
2 participants
Context
In the agent playground, adding a trigger on one revision made it show up on
every revision of that variant, including ones committed before it. Add a
trigger on revision X, open revision X-1, and the trigger was listed there too.
Cause: the Triggers section scoped a trigger by matching its
data.references(a variant-level id) against the agent's ids. Any trigger bound to the variant
therefore appeared on all of that variant's revisions.
Changes
Triggers created from the playground now record the revision they were added on
in the trigger's
meta:The section filters on that origin instead of the variant id alone. A trigger
shows on its origin revision and on later revisions of the same variant, and
never on earlier revisions or on sibling variants.
The runtime binding is untouched.
data.referencesstill points at the variant,which the backend resolves to the latest revision at fire time
(
triggers/service.py). The origin marker is display-only metadata. We did notswitch the reference to a specific revision, because a revision-level reference
pins the trigger to an immutable snapshot and breaks that latest-tracking.
Matching (
triggerScope.tsagentTriggerMatchesContext):with current version >= origin version.
back to the existing reference match, so their behavior is unchanged.
Tests
triggerScope.test.ts(7 cases): origin shows on the same and laterrevisions, hides on older revisions and on sibling variants, legacy
variant-bound triggers stay visible everywhere, and exact-revision references
still match.
pnpm --filter @agenta/entity-ui exec vitest runandpnpm turbo run build --filter=@agenta/entities --filter=@agenta/entity-uiboth pass.
What to QA
schedule) on the current revision. It appears in the Triggers section.
revision.
marker) still shows across the variant's revisions as before.
variant's do not.