Yicong-Huang commented on code in PR #6437:
URL: https://github.com/apache/texera/pull/6437#discussion_r3764522889
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -1717,6 +1762,158 @@ 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 handleOperatorRecommendation(): void {
+ this.repositionSuggestion$
+ .pipe(auditTime(100), untilDestroyed(this))
+ .subscribe(() => this.repositionRecommendations());
+
+ if (!this.operatorRecommendationService.isEnabled()) {
+ return;
+ }
+
+ // 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
Review Comment:
`dragDropped` links the dropped operator before it emits, on both branches —
`createEdgeReconnectionLinks` for a drop onto an existing edge
(drag-drop.service.ts:71), and `getNewOperatorLinks(..., suggestionOutputs)`
for proximity auto-linking (73).
So the chips can anchor on a port that already has a successor, and
`materialize`'s fixed +100 offset can land on top of it. Is suggesting after an
auto-link intended? If not, a `getAllLinks()` check on the source port skips it
cheaply.
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -1717,6 +1762,158 @@ 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 handleOperatorRecommendation(): void {
+ this.repositionSuggestion$
+ .pipe(auditTime(100), untilDestroyed(this))
+ .subscribe(() => this.repositionRecommendations());
+
+ if (!this.operatorRecommendationService.isEnabled()) {
+ return;
+ }
+
+ // 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.showRecommendationsFor(operator));
+
+ // Dismiss when the user clicks on blank canvas.
+ fromJointPaperEvent(this.paper, "blank:pointerdown")
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.closeRecommendations());
+
+ // Dismiss if the anchor operator is deleted out from under the
suggestions.
+ this.workflowActionService
+ .getTexeraGraph()
+ .getOperatorDeleteStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(({ deletedOperatorID }) => {
+ if (this.operatorSuggestion?.operatorId === deletedOperatorID) {
+ this.closeRecommendations();
+ }
+ });
+
+ // Keep the suggestions anchored to the operator's output port as it moves.
+ this.paper.model.on("change:position", (cell: joint.dia.Cell) => {
+ if (this.operatorSuggestion && cell.id.toString() ===
this.operatorSuggestion.operatorId) {
+ this.repositionSuggestion$.next();
+ }
+ });
+
+ // Keep the suggestions anchored on zoom.
+ this.wrapper
+ .getWorkflowEditorZoomStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => {
+ if (this.operatorSuggestion) {
+ this.repositionRecommendations();
+ }
+ });
+ }
+
+ private showRecommendationsFor(operator: OperatorPredicate): void {
+ this.closeRecommendations();
+ if (operator.outputPorts.length === 0) {
+ return;
+ }
+ const sourceOutputPortID = operator.outputPorts[0].portID;
+
+ this.operatorRecommendationService
+ .getRecommendations(operator)
Review Comment:
Reinforcing Copilot's cancellation point (r3600302103) — that thread was
resolved without a code change.
This subscription is never cancelled. Drop A then drop B: response A can
land last and show A's chips. Blank-click to dismiss: a late response re-opens
the overlay, because `closeRecommendations` (1885) only nulls the state.
I'd route the drop stream and `materializeRecommendation`'s chaining call
through one subject and `switchMap` it — that buys cancellation and ordering
together. A request token fixes only the first case.
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -109,6 +113,22 @@ export class WorkflowEditorComponent implements OnInit,
AfterViewInit, OnDestroy
position: { x: number; y: number };
} | null = null;
+ // Ambient operator recommender state (apache/texera#5240). Holds the faded
+ // next-operator suggestions anchored on the output port of the
+ // operator that was just added; null when nothing is being suggested.
+ public operatorSuggestion: {
Review Comment:
`DragDropService` already owns this vocabulary for a different feature:
`handleOperatorRecommendationOnDrag` (drag-drop.service.ts:114),
`operatorSuggestionHighlightStream`, `resetSuggestions`,
`SUGGESTION_DISTANCE_THRESHOLD` — proximity *link* suggestions. The new emit
sits three lines below that handler's call site.
Grepping either term now returns both features, indistinguishable. I'd
rename the component-side symbols to `nextOperatorSuggestion*`, keeping the
service and the wire types aligned with the backend's `OperatorRecommendation`.
Partly downstream of the ghost→suggestion rename, so a consequence of review
rather than an oversight.
##########
bin/single-node/.env:
##########
@@ -86,6 +86,7 @@
FILE_SERVICE_UPLOAD_ONE_FILE_TO_DATASET_ENDPOINT=http://file-service:9092/api/da
# Toggles the texera agent panel; set false to hide it in the GUI.
GUI_WORKFLOW_WORKSPACE_COPILOT_ENABLED=true
+GUI_WORKFLOW_WORKSPACE_OPERATOR_RECOMMENDATION_ENABLED=false
Review Comment:
This lands directly under the copilot comment with no separator, so `#
Toggles the texera agent panel` now reads as documenting both flags. Every
other group in the file gets its own comment block.
```suggestion
# Toggles the ambient operator recommender (next-operator suggestions on the
canvas).
GUI_WORKFLOW_WORKSPACE_OPERATOR_RECOMMENDATION_ENABLED=false
```
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.html:
##########
@@ -46,4 +46,24 @@
</div>
</div>
</div>
+
+ <!-- Ambient operator recommender: operator suggestions on the output port
-->
+ @if (operatorSuggestion) {
+ <div
+ class="operator-recommendation-container"
+ [style.left.px]="operatorSuggestion.position.x"
+ [style.top.px]="operatorSuggestion.position.y">
+ @for (recommendation of operatorSuggestion.recommendations; track
recommendation.operatorType) {
Review Comment:
`operatorType` comes from the backend, so it is not a guaranteed-unique
track key, and duplicates raise NG0955 — a hard render failure inside a feature
that swallows errors everywhere else. I can't confirm from this branch whether
the ranker can emit duplicates, but `$index` is free.
```suggestion
@for (recommendation of operatorSuggestion.recommendations; track
$index) {
```
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -1717,6 +1762,158 @@ 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 handleOperatorRecommendation(): void {
+ this.repositionSuggestion$
+ .pipe(auditTime(100), untilDestroyed(this))
+ .subscribe(() => this.repositionRecommendations());
+
+ if (!this.operatorRecommendationService.isEnabled()) {
+ return;
+ }
+
+ // 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.showRecommendationsFor(operator));
+
+ // Dismiss when the user clicks on blank canvas.
+ fromJointPaperEvent(this.paper, "blank:pointerdown")
+ .pipe(untilDestroyed(this))
+ .subscribe(() => this.closeRecommendations());
+
+ // Dismiss if the anchor operator is deleted out from under the
suggestions.
+ this.workflowActionService
+ .getTexeraGraph()
+ .getOperatorDeleteStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(({ deletedOperatorID }) => {
+ if (this.operatorSuggestion?.operatorId === deletedOperatorID) {
+ this.closeRecommendations();
+ }
+ });
+
+ // Keep the suggestions anchored to the operator's output port as it moves.
+ this.paper.model.on("change:position", (cell: joint.dia.Cell) => {
+ if (this.operatorSuggestion && cell.id.toString() ===
this.operatorSuggestion.operatorId) {
+ this.repositionSuggestion$.next();
+ }
+ });
+
+ // Keep the suggestions anchored on zoom.
+ this.wrapper
+ .getWorkflowEditorZoomStream()
+ .pipe(untilDestroyed(this))
+ .subscribe(() => {
+ if (this.operatorSuggestion) {
+ this.repositionRecommendations();
+ }
+ });
+ }
+
+ private showRecommendationsFor(operator: OperatorPredicate): void {
+ this.closeRecommendations();
+ if (operator.outputPorts.length === 0) {
+ return;
+ }
+ const sourceOutputPortID = operator.outputPorts[0].portID;
+
+ this.operatorRecommendationService
+ .getRecommendations(operator)
+ .pipe(untilDestroyed(this))
+ .subscribe(recommendations => {
+ // The operator may have been deleted while the request was in flight.
+ if (
+ recommendations.length === 0 ||
+
!this.workflowActionService.getTexeraGraph().hasOperator(operator.operatorID)
+ ) {
+ return;
+ }
+ const position = this.getRecommendationPosition(operator.operatorID);
+ if (!position) {
+ return;
+ }
+ this.operatorSuggestion = {
+ operatorId: operator.operatorID,
+ sourceOutputPortID,
+ 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.
+ */
+ materializeRecommendation(recommendation: OperatorRecommendation): void {
+ if (!this.operatorSuggestion) {
+ return;
+ }
+ const graph = this.workflowActionService.getTexeraGraph();
+ let newOperatorID: string | undefined;
+ if (graph.hasOperator(this.operatorSuggestion.operatorId)) {
+ const sourceOperator =
graph.getOperator(this.operatorSuggestion.operatorId);
+ newOperatorID = this.operatorRecommendationService.materialize(
+ sourceOperator,
+ this.operatorSuggestion.sourceOutputPortID,
+ recommendation.operatorType
+ );
+ }
+ this.closeRecommendations();
+
+ // Chaining has to be explicit: a materialized operator reaches the graph
+ // through addOperatorsAndLinks, not through a drop, so the drop-gated
+ // trigger above will never fire for it.
+ if (newOperatorID !== undefined && graph.hasOperator(newOperatorID)) {
+ this.showRecommendationsFor(graph.getOperator(newOperatorID));
+ }
+ }
+
+ closeRecommendations(): void {
Review Comment:
No template binding for this one — the markup only calls
`materializeRecommendation`.
```suggestion
private closeRecommendations(): void {
```
##########
bin/k8s/templates/base/gateway/gateway-routes.yaml:
##########
@@ -159,6 +159,35 @@ spec:
port: 3001
{{- end }}
---
+# Operator Recommender Route (apache/texera#5240). Served by the agent-service,
+# but deliberately kept out of the agent-service HTTPRoute: that route is the
+# target of the agent BackendTrafficPolicy, which pins requests to a replica by
+# consistent-hashing X-Agent-Workflow-Id. The recommender is stateless and
sends
+# no such header, so it must not inherit that affinity. Longest-prefix matching
Review Comment:
The recommender *receives* these requests; it is the frontend client that
omits the header. Worth being exact, since this comment is the entire
justification for splitting the route out.
```suggestion
# consistent-hashing X-Agent-Workflow-Id. The recommender is stateless and
its
# requests carry no such header, so it must not inherit that affinity.
Longest-prefix matching
```
--
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]
