Yicong-Huang commented on code in PR #6437:
URL: https://github.com/apache/texera/pull/6437#discussion_r3769696258


##########
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:
   The request-time half is verified — `portIsTaken` at 1854-1861, with a test 
at spec:1717. Leaving this open for the other half, and for one correction.
   
   The click path has no equivalent check. `materializeNextOperatorSuggestion` 
re-reads the source operator and re-checks `hasOperator` (1902) but never 
re-checks the port, and nothing dismisses the chips when a link appears on it — 
link drags start on a port, so they fire `element:pointerdown`, not the 
`blank:pointerdown` at 1809 that dismisses. Draw a link out of the anchor port 
while the chips are up, then click a chip, and you get the second successor at 
the same fixed offset. Separate comment on 1902.
   
   The correction is to your second note: the code only ever looks at 
`outputPorts[0]` — at 1854 for the guard and 1883 for the rendered state — so a 
multi-output operator whose *first* port is wired gets no suggestions at all, 
rather than suggestions on a free one. Conservative and fine as behaviour; just 
not what the note describes.



-- 
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]

Reply via email to