Ma77Ball commented on code in PR #8637:
URL: https://github.com/apache/texera/pull/8637#discussion_r4107521354
##########
demo-workflows/README.md:
##########
@@ -0,0 +1,59 @@
+# Demo workflows for the AI Workflow Fixer
Review Comment:
This README has no Apache license header, so the `check-header` CI job
(`apache/skywalking-eyes`) will fail on this PR.
Why it matters: `.licenserc.yaml` ignores `**/*.json` but not `**/*.md`, and
every existing README.md in the repo carries the ASF header. Without it, the
required license check goes red and blocks merge. (If you drop the folder per
the other comment, this is moot.)
Suggested fix:
```suggestion
<!--
~ Licensed to the Apache Software Foundation (ASF) under one
~ or more contributor license agreements. See the NOTICE file
~ distributed with this work for additional information
~ regarding copyright ownership. The ASF licenses this file
~ to you under the Apache License, Version 2.0 (the
~ "License"); you may not use this file except in compliance
~ with the License. You may obtain a copy of the License at
~
~ http://www.apache.org/licenses/LICENSE-2.0
~
~ Unless required by applicable law or agreed to in writing,
~ software distributed under the License is distributed on an
~ "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
~ KIND, either express or implied. See the License for the
~ specific language governing permissions and limitations
~ under the License.
-->
# Demo workflows for the AI Workflow Fixer
```
##########
demo-workflows/combined.json:
##########
@@ -0,0 +1,92 @@
+{
Review Comment:
This PR adds a new top-level `demo-workflows/` directory (README + 7 JSON
workflows, ~800 lines) that no test, build, or runtime code references.
Why it matters: it roughly doubles the PR's line count with material
unrelated to the feature code, 5 of the 7 files need the external movies
dataset to run, and it adds a new top-level path to the repo for demo
artifacts. The JSON operator shapes will also drift from the engine over time
with nothing to catch it.
Suggested fix: drop the folder from this PR (you already offered to). If you
want runnable examples kept, the README notes they share the shape of
`bin/single-node/examples/workflows/`, so move them there rather than adding a
new top-level directory. Not a functional blocker, but it should not merge
as-is at repo root.
##########
frontend/src/app/workspace/component/result-panel/ai-fix/ai-fix-frame.component.ts:
##########
@@ -0,0 +1,132 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Component, Input } from "@angular/core";
+import { AsyncPipe, NgFor, NgIf } from "@angular/common";
+import { Observable } from "rxjs";
+import { NzAlertComponent } from "ng-zorro-antd/alert";
+import { NzSpinComponent } from "ng-zorro-antd/spin";
+import { NzTagComponent } from "ng-zorro-antd/tag";
+import { NzButtonComponent } from "ng-zorro-antd/button";
+import { NzWaveDirective } from "ng-zorro-antd/core/wave";
+import { NzIconDirective } from "ng-zorro-antd/icon";
+import { ExecuteWorkflowService } from
"../../../service/execute-workflow/execute-workflow.service";
+import { WorkflowActionService } from
"../../../service/workflow-graph/model/workflow-action.service";
+import { WorkflowCompilingService } from
"../../../service/compile-workflow/workflow-compiling.service";
+import { WorkflowConsoleService } from
"../../../service/workflow-console/workflow-console.service";
+import { AiWorkflowFixerService, FixConfidence, FixState, UNSUPPORTED_MESSAGE
} from "./ai-workflow-fixer.service";
+
+/**
+ * Result-panel frame that asks the LLM to fix the failed operator.
+ *
+ * The analysis is started by an explicit button: ResultPanelComponent rebuilds
+ * every frame on each re-render (clearResultPanel), so analysing on init would
+ * fire a model request per re-render. For the same reason all state lives in
+ * AiWorkflowFixerService and this component keeps none.
+ */
+@Component({
+ selector: "texera-ai-fix-frame",
+ templateUrl: "./ai-fix-frame.component.html",
+ styleUrls: ["./ai-fix-frame.component.scss"],
+ imports: [
+ NgIf,
+ NgFor,
+ AsyncPipe,
+ NzAlertComponent,
+ NzSpinComponent,
+ NzTagComponent,
+ NzButtonComponent,
+ NzWaveDirective,
+ NzIconDirective,
+ ],
+})
+export class AiFixFrameComponent {
+ @Input() operatorId?: string;
+
+ public readonly state$: Observable<FixState>;
+ public readonly unsupportedMessage = UNSUPPORTED_MESSAGE;
+
+ constructor(
+ private aiWorkflowFixerService: AiWorkflowFixerService,
+ private executeWorkflowService: ExecuteWorkflowService,
+ private workflowActionService: WorkflowActionService,
+ private workflowCompilingService: WorkflowCompilingService,
+ private workflowConsoleService: WorkflowConsoleService
+ ) {
+ this.state$ = aiWorkflowFixerService.getState$();
Review Comment:
The frame binds the shared service state without checking it belongs to this
frame's operator, so a suggestion analyzed for one operator can be shown and
applied under another.
Why it matters: `AiWorkflowFixerService` is a root singleton with one
`stateSubject`, and the template renders `state$ | async` with no comparison to
`@Input() operatorId`. Concrete sequence: two Python UDFs in a pipeline both
raise an ERROR console message, so both get an AI Fix tab. Analyze operator A
-> state = {operatorId: A, status: ready, suggestedFix: fixA}. Select operator
B: the panel rebuilds the frame with `operatorId = B`, but `state$` still holds
A's ready suggestion, so B's tab shows A's root cause and diff. Clicking Apply
calls `applyFix()`, which reads `current.operatorId` (= A) and patches operator
A while the user is looking at B, then re-runs. The write is undoable but
targets the wrong operator.
Suggested fix: gate the ready/diff UI and the Apply/Discard actions on
`state.operatorId === operatorId` (expose a guarded observable, or reset the
service to idle when the bound `operatorId` differs from `state.operatorId`).
This is a multi-spot change across the component and template, not a one-click
edit. Worth a spec that analyzes one operator and asserts a second operator's
frame does not show or apply it.
##########
frontend/src/app/workspace/component/result-panel/ai-fix/ai-workflow-fixer.service.ts:
##########
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Injectable } from "@angular/core";
+import { BehaviorSubject, Observable } from "rxjs";
+import { createOpenAI } from "@ai-sdk/openai";
+import { generateText, type ModelMessage } from "ai";
+import { AppSettings } from "../../../../common/app-setting";
+import { AuthService } from "../../../../common/service/user/auth.service";
+import { WorkflowActionService } from
"../../../service/workflow-graph/model/workflow-action.service";
+import { ExecuteWorkflowService } from
"../../../service/execute-workflow/execute-workflow.service";
+import { WarehouseService } from
"../../../../common/service/warehouse/warehouse.service";
+import { GuiConfigService } from
"../../../../common/service/gui-config.service";
+import { PortSchema } from "../../../types/workflow-compiling.interface";
+import { buildFixPrompt } from "./ai-fix-prompt";
+
+export type FixErrorType = "missing_column" | "type_error" | "null_error" |
"model_not_found" | "unsupported";
+export type FixConfidence = "high" | "medium" | "low";
+
+export interface SuggestedFix {
+ type: "code_change" | "property_change";
+ original: string;
+ suggested: string;
+ explanation: string;
+ // Rendered as the confidence badge; `fieldName` names the patched property
+ // (property_change only) so applyFix knows which field to write.
+ confidence: FixConfidence;
+ fieldName?: string;
+}
+
+export interface FixState {
+ operatorId: string;
+ errorMessage: string;
+ errorType: FixErrorType;
+ originalCode?: string;
+ originalProperties?: object;
+ suggestedFix?: SuggestedFix;
+ // `applied_without_run`: the fix was written but the deployment refused to
start a
+ // run (a warehouse is required and none is selected), so the panel must not
claim one.
+ status: "idle" | "analyzing" | "ready" | "applying" | "applied" |
"applied_without_run" | "error";
+}
+
+// Model id as LiteLLM exposes it (bin/single-node/litellm-config.yaml), not
the provider's id.
+export const AI_FIXER_MODEL = "claude-haiku-4.5";
Review Comment:
`AI_FIXER_MODEL` is hard-coded to the LiteLLM alias `claude-haiku-4.5`.
Why it matters: any deployment whose `litellm-config.yaml` exposes a
different alias (or renames this one) gets a 404 from the proxy, which surfaces
as the generic "The AI could not suggest a fix" error with no hint that the
cause is a model-name mismatch. The frontend constant is silently coupled to
backend config.
Suggested fix: source the model id from `GuiConfig`/env (the same place
`copilotEnabled` comes from) instead of a compile-time constant, so it tracks
the proxy config. Optional, but it will bite a differently-configured
deployment.
##########
frontend/src/app/workspace/component/result-panel/ai-fix/ai-workflow-fixer.service.ts:
##########
@@ -0,0 +1,249 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { Injectable } from "@angular/core";
+import { BehaviorSubject, Observable } from "rxjs";
+import { createOpenAI } from "@ai-sdk/openai";
+import { generateText, type ModelMessage } from "ai";
+import { AppSettings } from "../../../../common/app-setting";
+import { AuthService } from "../../../../common/service/user/auth.service";
+import { WorkflowActionService } from
"../../../service/workflow-graph/model/workflow-action.service";
+import { ExecuteWorkflowService } from
"../../../service/execute-workflow/execute-workflow.service";
+import { WarehouseService } from
"../../../../common/service/warehouse/warehouse.service";
+import { GuiConfigService } from
"../../../../common/service/gui-config.service";
+import { PortSchema } from "../../../types/workflow-compiling.interface";
+import { buildFixPrompt } from "./ai-fix-prompt";
+
+export type FixErrorType = "missing_column" | "type_error" | "null_error" |
"model_not_found" | "unsupported";
+export type FixConfidence = "high" | "medium" | "low";
+
+export interface SuggestedFix {
+ type: "code_change" | "property_change";
+ original: string;
+ suggested: string;
+ explanation: string;
+ // Rendered as the confidence badge; `fieldName` names the patched property
+ // (property_change only) so applyFix knows which field to write.
+ confidence: FixConfidence;
+ fieldName?: string;
+}
+
+export interface FixState {
+ operatorId: string;
+ errorMessage: string;
+ errorType: FixErrorType;
+ originalCode?: string;
+ originalProperties?: object;
+ suggestedFix?: SuggestedFix;
+ // `applied_without_run`: the fix was written but the deployment refused to
start a
+ // run (a warehouse is required and none is selected), so the panel must not
claim one.
+ status: "idle" | "analyzing" | "ready" | "applying" | "applied" |
"applied_without_run" | "error";
+}
+
+// Model id as LiteLLM exposes it (bin/single-node/litellm-config.yaml), not
the provider's id.
+export const AI_FIXER_MODEL = "claude-haiku-4.5";
+export const AI_FIXER_TIMEOUT_MS = 120_000;
+export const UNSUPPORTED_MESSAGE = "This error type is not yet supported for
automatic fixing.";
+
+const IDLE_STATE: FixState = { operatorId: "", errorMessage: "", errorType:
"unsupported", status: "idle" };
+
+// Each pattern matches a line anywhere inside a traceback, not the whole
message.
+const PATTERNS: ReadonlyArray<[RegExp, FixErrorType]> = [
+ [/KeyError:\s*['"][^'"]+['"]/, "missing_column"],
+ [/TypeError:\s*unsupported operand/i, "type_error"],
+ [/ValueError:[^\n]*\bNaN\b|NullPointerException/i, "null_error"],
+ [/ModelNotFound|\b404\b/i, "model_not_found"],
Review Comment:
The `model_not_found` pattern matches a bare `404` anywhere in the traceback
via `\b404\b`.
Why it matters: a traceback that merely contains `404` (a line number, an
HTTP status from unrelated user code, a literal in the data) is classified as
`model_not_found` and sent to the model asking it to correct the model config
field, producing an irrelevant `property_change` suggestion. Low harm (one
wrong call), but the false-positive surface is wide.
Suggested fix: tie `404` to the model-lookup context rather than matching it
standalone (require it alongside a model / not-found token, or match the
specific message shape LiteLLM returns). Prose only: I do not know the exact
upstream error strings to give a precise regex.
--
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]
