Yicong-Huang commented on code in PR #6437:
URL: https://github.com/apache/texera/pull/6437#discussion_r3787852640
##########
frontend/src/app/workspace/service/joint-ui/joint-ui.service.ts:
##########
@@ -1080,3 +1080,20 @@ export function fromJointPaperEvent<T extends keyof
joint.dia.Paper.EventMap = k
(handler, signal) => paper.off(eventName as string, handler, context) //
removeHandler
);
}
+
+/**
+ * Observable of a JointJS graph's cell events (`change:position`, `add`, ...),
+ * emitting the cell the event fired on.
+ *
+ * The graph belongs to `JointGraphWrapper`, which is root-provided, so it
+ * outlives the components listening to it. Going through an Observable lets a
+ * listener be torn down with `untilDestroyed(this)` like any other
subscription,
+ * where a raw `graph.on(...)` closure would leak.
+ */
+export function fromJointGraphCellEvent(graph: joint.dia.Graph, eventName:
string): Observable<joint.dia.Cell> {
Review Comment:
`fromJointPaperEvent` two lines up constrains its event name to `keyof
joint.dia.Paper.EventMap`, so a wrong name is a compile error. This one takes a
bare `string` while asserting a `Cell` payload. Backbone also emits graph-level
events (`change`, `reset`, `sort`) that hand you the Graph, and the signature
does not refuse them.
No caller hits that today. It matters because the two raw listeners at
`workflow-editor.component.ts:501` and `1678` are the obvious next ones.
```suggestion
export function fromJointGraphCellEvent(
graph: joint.dia.Graph,
eventName: "add" | "remove" | "change:position"
): Observable<joint.dia.Cell> {
```
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -1717,6 +1769,221 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
return
this.operatorSummaries.get(operatorId)?.sampleRecords?.[0]?.["__is_visualization__"]
=== true;
}
+ /**
+ * Ambient operator recommender (apache/texera#5240). When the user drops an
+ * operator onto the canvas, ask the recommender for likely next operators
and
+ * float them as suggestion chips on the operator's output port; clicking one
+ * materializes it. The whole feature is opt-in and self-effacing: if it is
+ * disabled or the backend returns nothing, the canvas is untouched.
+ */
+ private handleNextOperatorSuggestions(): void {
+ if (!this.operatorRecommendationService.isEnabled()) {
+ return;
+ }
+
+ // Repositioning is throttled: change:position fires once per drag frame.
+ this.repositionNextOperatorSuggestion$
+ .pipe(auditTime(100), untilDestroyed(this))
+ .subscribe(() => this.repositionNextOperatorSuggestions());
+
+ // Every suggestion request — from a drop or from chaining after a click —
+ // goes through this one pipeline. switchMap unsubscribes the previous
+ // request, so a slow response can neither overwrite newer suggestions nor
+ // re-open the overlay after the user dismissed it; `null` means "cancel".
+ this.nextOperatorSuggestionRequest$
+ .pipe(
+ switchMap(operator =>
+ operator === null
+ ? of(null)
+ : this.operatorRecommendationService
+ .getRecommendations(operator)
+ .pipe(map(recommendations => ({ operator, recommendations })))
+ ),
+ untilDestroyed(this)
+ )
+ .subscribe(result => this.showNextOperatorSuggestions(result));
+
+ // Trigger: the user interactively dropped an operator onto the canvas.
+ // Deliberately not the graph's operator-add stream, which also fires on
+ // workflow load, undo/redo, paste, and remote co-editor edits — none of
+ // which are a user authoring a next step.
+ this.dragDropService.operatorDropStream
+ .pipe(untilDestroyed(this))
+ .subscribe(operator => this.requestNextOperatorSuggestionsFor(operator));
+
+ // Dismiss when the user clicks on blank canvas.
+ fromJointPaperEvent(this.paper, "blank:pointerdown")
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.closeNextOperatorSuggestions());
+
+ // Dismiss when the canvas is frozen — during execution, version preview,
or a
+ // read-only view. Materializing writes to the shared model, and undo
refuses
+ // while the lock is on, so a click here would be unrepealable; the chips
must
Review Comment:
`repeal` applies to statutes, not to a canvas edit. The point the clause
before it makes is that `undoAction()` refuses while the lock is on, so the
write cannot be taken back.
```suggestion
// while the lock is on, so a click here could not be undone; the chips
must
```
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -1717,6 +1769,221 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
return
this.operatorSummaries.get(operatorId)?.sampleRecords?.[0]?.["__is_visualization__"]
=== true;
}
+ /**
+ * Ambient operator recommender (apache/texera#5240). When the user drops an
+ * operator onto the canvas, ask the recommender for likely next operators
and
+ * float them as suggestion chips on the operator's output port; clicking one
+ * materializes it. The whole feature is opt-in and self-effacing: if it is
+ * disabled or the backend returns nothing, the canvas is untouched.
+ */
+ private handleNextOperatorSuggestions(): void {
+ if (!this.operatorRecommendationService.isEnabled()) {
+ return;
+ }
+
+ // Repositioning is throttled: change:position fires once per drag frame.
+ this.repositionNextOperatorSuggestion$
+ .pipe(auditTime(100), untilDestroyed(this))
+ .subscribe(() => this.repositionNextOperatorSuggestions());
+
+ // Every suggestion request — from a drop or from chaining after a click —
+ // goes through this one pipeline. switchMap unsubscribes the previous
+ // request, so a slow response can neither overwrite newer suggestions nor
+ // re-open the overlay after the user dismissed it; `null` means "cancel".
+ this.nextOperatorSuggestionRequest$
+ .pipe(
+ switchMap(operator =>
+ operator === null
+ ? of(null)
+ : this.operatorRecommendationService
+ .getRecommendations(operator)
+ .pipe(map(recommendations => ({ operator, recommendations })))
+ ),
+ untilDestroyed(this)
+ )
+ .subscribe(result => this.showNextOperatorSuggestions(result));
+
+ // Trigger: the user interactively dropped an operator onto the canvas.
+ // Deliberately not the graph's operator-add stream, which also fires on
+ // workflow load, undo/redo, paste, and remote co-editor edits — none of
+ // which are a user authoring a next step.
+ this.dragDropService.operatorDropStream
+ .pipe(untilDestroyed(this))
+ .subscribe(operator => this.requestNextOperatorSuggestionsFor(operator));
+
+ // Dismiss when the user clicks on blank canvas.
+ fromJointPaperEvent(this.paper, "blank:pointerdown")
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.closeNextOperatorSuggestions());
+
+ // Dismiss when the canvas is frozen — during execution, version preview,
or a
+ // read-only view. Materializing writes to the shared model, and undo
refuses
+ // while the lock is on, so a click here would be unrepealable; the chips
must
+ // not stay on screen looking clickable either.
+ this.workflowActionService
+ .getWorkflowModificationEnabledStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(enabled => {
+ if (!enabled) {
+ this.closeNextOperatorSuggestions();
+ }
+ });
+
+ // Dismiss if the anchor operator is deleted out from under the
suggestions.
+ this.workflowActionService
+ .getTexeraGraph()
+ .getOperatorDeleteStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(({ deletedOperatorID }) => {
+ if (this.nextOperatorSuggestion?.operatorId === deletedOperatorID) {
+ this.closeNextOperatorSuggestions();
+ }
+ });
+
+ // Keep the suggestions anchored to the operator's output port as it moves.
+ fromJointGraphCellEvent(this.paper.model, "change:position")
+ .pipe(untilDestroyed(this))
+ .subscribe(cell => {
+ if (this.nextOperatorSuggestion && cell.id.toString() ===
this.nextOperatorSuggestion.operatorId) {
+ this.repositionNextOperatorSuggestion$.next();
+ }
+ });
+
+ // Keep the suggestions anchored on zoom.
+ this.wrapper
+ .getWorkflowEditorZoomStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => {
+ if (this.nextOperatorSuggestion) {
+ this.repositionNextOperatorSuggestions();
+ }
+ });
+ }
+
+ /** Ask for suggestions on `operator`, cancelling whatever was in flight. */
+ private requestNextOperatorSuggestionsFor(operator: OperatorPredicate): void
{
+ this.closeNextOperatorSuggestions();
+ // An operator with no output ports (e.g. a chart sink) has no port to
+ // anchor suggestions on, so there is nothing to ask for.
+ if (operator.outputPorts.length === 0) {
+ return;
+ }
+ // A drop can arrive already wired: onto an existing edge, or auto-linked
to
+ // a nearby operator. The next step is chosen in that case, so suggesting
+ // another one would both be noise and risk placing it on top of the
+ // successor the drop just created.
+ if (this.isOutputPortLinked(operator.operatorID,
operator.outputPorts[0].portID)) {
+ return;
+ }
+ this.nextOperatorSuggestionRequest$.next(operator);
+ }
+
+ /** Whether `portID` on `operatorID` already has a link leaving it. */
+ private isOutputPortLinked(operatorID: string, portID: string): boolean {
+ return this.workflowActionService
+ .getTexeraGraph()
+ .getAllLinks()
+ .some(link => link.source.operatorID === operatorID &&
link.source.portID === portID);
+ }
+
+ /** Render the result of the most recent, uncancelled suggestion request. */
+ private showNextOperatorSuggestions(
+ result: { operator: OperatorPredicate; recommendations:
OperatorRecommendation[] } | null
+ ): void {
+ if (result === null || result.recommendations.length === 0) {
+ return;
+ }
+ const { operator, recommendations } = result;
+ // The operator may have been deleted while the request was in flight.
+ if
(!this.workflowActionService.getTexeraGraph().hasOperator(operator.operatorID))
{
+ return;
+ }
+ const position =
this.getNextOperatorSuggestionPosition(operator.operatorID);
+ if (!position) {
+ return;
+ }
+ this.nextOperatorSuggestion = {
+ operatorId: operator.operatorID,
+ sourceOutputPortID: operator.outputPorts[0].portID,
+ position,
+ recommendations,
+ };
+ this.changeDetectorRef.detectChanges();
+ }
+
+ /**
+ * Materialize a clicked suggestion into a real operator wired onto the
+ * source operator's output port, then suggest what could follow that new
+ * operator in turn — accepting a suggestion leaves the canvas ready for the
+ * next one, the way accepting a code completion does.
+ */
+ materializeNextOperatorSuggestion(recommendation: OperatorRecommendation):
void {
+ if (!this.nextOperatorSuggestion) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ let newOperatorID: string | undefined;
+ // Re-check the port: nothing dismisses the chips when the user hand-draws
a
+ // link out of the anchor port, because that gesture starts on the port and
+ // fires element:pointerdown rather than the blank:pointerdown we listen
for.
+ // Without this the click would add a second successor on top of the first.
Review Comment:
The event name is wrong, and the comment no longer covers the whole
condition.
A port drag never fires `element:pointerdown`. In jointjs 3.5.4
`ElementView.onmagnet` calls `dragMagnetStart`, which stops propagation and
hands off to `dragLinkStart`. `notifyPointerdown`, the only emitter of that
event, is never reached, and there is no `element:magnet:pointerdown` either.
Your conclusion holds and the guard is right — only the stated mechanism is off.
The condition also leads with `checkWorkflowModificationEnabled()` now,
which this comment does not mention.
```suggestion
// Both conditions re-check state the chips cannot see. The lock can
engage
// between render and click; and nothing dismisses the chips when the
user
// hand-draws a link out of the anchor port, because that gesture is a
magnet
// drag that stops propagation before any pointerdown notification, so
the
// blank:pointerdown we listen for never fires. Without the port check
the
// click would add a second successor on top of the first.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
