Yicong-Huang commented on code in PR #6437:
URL: https://github.com/apache/texera/pull/6437#discussion_r3769747861
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -1717,6 +1764,194 @@ 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 {
+ this.repositionNextOperatorSuggestion$
+ .pipe(auditTime(100), untilDestroyed(this))
+ .subscribe(() => this.repositionNextOperatorSuggestions());
Review Comment:
This subscription sits above the `isEnabled()` return two lines down. With
the flag off it stays alive with nothing able to feed it — the subject's only
producer is the `change:position` handler at 1825, registered below that return.
Harmless today, but it reads as though repositioning still works when the
feature is off. Moving it under the return puts the whole feature behind one
gate.
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.ts:
##########
@@ -1717,6 +1764,194 @@ 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 {
+ this.repositionNextOperatorSuggestion$
+ .pipe(auditTime(100), untilDestroyed(this))
+ .subscribe(() => this.repositionNextOperatorSuggestions());
+
+ if (!this.operatorRecommendationService.isEnabled()) {
+ return;
+ }
+
+ // 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 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.
+ this.paper.model.on("change:position", (cell: joint.dia.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.
+ const sourcePortID = operator.outputPorts[0].portID;
+ const portIsTaken = this.workflowActionService
+ .getTexeraGraph()
+ .getAllLinks()
+ .some(link => link.source.operatorID === operator.operatorID &&
link.source.portID === sourcePortID);
+ if (portIsTaken) {
+ return;
+ }
+ this.nextOperatorSuggestionRequest$.next(operator);
+ }
+
+ /** 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;
+ if (graph.hasOperator(this.nextOperatorSuggestion.operatorId)) {
Review Comment:
The new guard covers the ask path but not this one. `portIsTaken` runs
before the request (1854); by the time a chip is clicked, the port can have
acquired a link.
Nothing dismisses the chips when that happens — a link drag starts on a
port, so it fires `element:pointerdown`, not the `blank:pointerdown` at 1809.
Drop an operator, hand-draw a link out of its output port, then click a chip.
You get the second successor at the fixed +100 offset you agreed was worse than
noise.
Re-running the same `getAllLinks()` check here is the fix I'd take —
dismissing on a link-add from the anchor port is more thorough but costs a
subscription for a narrow window.
##########
frontend/src/app/workspace/component/workflow-editor/workflow-editor.component.spec.ts:
##########
@@ -1644,3 +1649,155 @@ describe("WorkflowEditorComponent link breakpoints", ()
=> {
expect(show.mock.invocationCallOrder[0]).toBeLessThan(hide.mock.invocationCallOrder[0]);
});
});
+
+/**
+ * Ambient operator recommender (apache/texera#5240).
+ *
+ * The feature is opt-in and `handleNextOperatorSuggestions` reads the flag
once
+ * during ngAfterViewInit, returning early when it is off — so the flag has to
+ * be set before the first change detection, not inside the individual tests.
+ */
+describe("Ambient Operator Recommender", () => {
+ let component: WorkflowEditorComponent;
+ let fixture: ComponentFixture<WorkflowEditorComponent>;
+ let workflowActionService: WorkflowActionService;
+ let dragDropService: DragDropService;
+ let getRecommendationsSpy: MockInstance;
+
+ const mockRecommendations = [{ operatorType: "NlpSentiment", score: 0.9,
reason: "Analyze the text" }];
+
+ // The recommender listens to the drag-drop service's drop stream. Push the
+ // dropped operator through directly rather than driving a real drag gesture,
+ // which would need a live flying-operator paper.
+ function emitOperatorDrop(operator: OperatorPredicate): void {
+ (dragDropService as any).operatorDroppedSubject.next(operator);
+ }
+
+ beforeEach(async () => {
+ await TestBed.configureTestingModule({
+ imports: workflowEditorTestImports,
+ providers: workflowEditorTestProviders,
+ }).compileComponents();
+ });
+
+ beforeEach(() => {
+ fixture = TestBed.createComponent(WorkflowEditorComponent);
+ component = fixture.componentInstance;
+ workflowActionService = TestBed.inject(WorkflowActionService);
+ dragDropService = TestBed.inject(DragDropService);
+
+ (TestBed.inject(GuiConfigService).env as
any).operatorRecommendationEnabled = true;
+ getRecommendationsSpy = vi
+ .spyOn(TestBed.inject(OperatorRecommendationService),
"getRecommendations")
+ .mockReturnValue(of(mockRecommendations));
+
+ // detect changes to run ngAfterViewInit and wire up the recommender
+ fixture.detectChanges();
+ });
+
+ it("requests suggestions when the user drops an operator onto the canvas",
() => {
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+ emitOperatorDrop(mockScanPredicate);
+
+ expect(getRecommendationsSpy).toHaveBeenCalledTimes(1);
+
expect(component.nextOperatorSuggestion?.operatorId).toEqual(mockScanPredicate.operatorID);
+
expect(component.nextOperatorSuggestion?.recommendations).toEqual(mockRecommendations);
+ });
+
+ it("does not request suggestions for operators that arrive without a drop",
() => {
+ // Workflow load, undo/redo, paste and remote co-editor edits all reach the
+ // graph through addOperator with no drop event, and must stay silent.
+ workflowActionService.addOperator(mockScanPredicate, mockPoint);
+ workflowActionService.addOperator(mockSentimentPredicate, mockPoint);
+
+ expect(getRecommendationsSpy).not.toHaveBeenCalled();
+ expect(component.nextOperatorSuggestion).toBeNull();
+ });
+
+ it("does not suggest when the drop already wired the output port", () => {
+ // dragDropped links on both branches — dropping onto an existing edge, and
Review Comment:
`links` is doing verb duty with no object, so the sentence never says what
`dragDropped` links.
```suggestion
// dragDropped adds links on both branches — dropping onto an existing
edge, and
```
--
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]
