[pedroreyesalvarado]

What changes were proposed in this PR?

Adds an AI Fix tab to the result panel. When a Python UDF operator fails, the user clicks Analyze with AI; the tab sends the error, the operator's code, its input schemas and its configuration to an LLM through the existing LiteLLM proxy (/api/chat/completions), and shows the proposed fix as a diff with a confidence badge. Apply and re-run patches the operator through setOperatorProperty (so it is undoable) and re-executes the workflow; Discard drops it.

It handles four error shapes and says so plainly for anything else ("This error type is not yet supported for automatic fixing."):

Error	Fix the model is asked for
KeyError: 'col'	closest column name from the input schema
TypeError: unsupported operand	an explicit cast
ValueError: ... NaN / NullPointerException	.dropna() or a null guard
model not found / 404 beside a model reference	the corrected model field in the operator config

Classification reads the final exception line of the traceback, not the whole message: the frame lines carry paths and line numbers that look like patterns on their own, and a chained traceback reports the handled exception before the one that stopped the worker.

What leaves the browser. The error text, the operator's code, every input port's schema, and its configuration minus any credential. Operators keep secrets beside ordinary settings — hfApiToken on HuggingFaceInference, password on the SQL, Reddit and Twitter sources — so those fields are dropped before the prompt is built, behind two independent barriers: the operator's own schema marks them with the password widget, and a name pattern catches a schema that cannot be read or an operator missing the annotation. The UDF source is sent once, fenced under its own heading: code is a property like any other, so without dropping it from the configuration block the whole operator source travelled twice, and the JSON-escaped copy is the one no snippet can be quoted from.

When the tab appears. A Python UDF exception never becomes a WorkflowFatalError: the worker pauses and the traceback arrives as an ERROR ConsoleMessage while the execution stays Running. The tab is driven by the console stream for that reason, with its own subscription that adds or removes only that entry — a full panel rerender on every console update would reset the Console frame's filter, scroll and debug input and the Result table whenever any operator printed. The frame tabs are keyed by name for the same reason, because keyvalue hands *ngFor a fresh pair object per entry: without that the narrowed subscription still rebuilt every tab whenever one was added. It tears itself down when the operator stops reporting an error, and the whole panel resets when a run ends — or, since a UDF that raises leaves the run Running and never reaches a terminal state, when a newer error arrives on the same operator.

What Apply guarantees.

• The operator is re-read from the graph first, and the write is refused if the code or the field has moved on since the suggestion was generated.
• A suggestion that cannot be applied never reaches the screen: a snippet the model invented (zero matches in the code), a field the operator does not have, and code itself are all refused at the analysis, not at the click. For a property change the diff's "before" is the field's current value, so those would otherwise render an empty diff — or, for code, the whole UDF source as one unwrapped line.
• The model's string is written back in the type the field already held, and a configuration change is only accepted for the one error the prompt asks one for.
• Every occurrence of the snippet is replaced, since a re-run would otherwise fail again on the second one; the panel says how many places that is when it is more than one.
• A multi-line suggestion replacing a single-line match has its continuation lines indented to the line the match was found on. A match that already spans lines carries the absolute indentation of the region it came from, and the reply mirrors it, so nothing is added.
• The modification lock is cleared first. The traceback arrives while the execution is still Running, and Texera locks workflow modification for as long as a run is live, so the panel kills the paused execution — which also releases the computing unit it still held. It refuses instead of killing when a live run is not what holds the lock: a read-only workflow locks through a path enableWorkflowModification() cannot reopen, and running only needs write access to the computing unit, so a read-only user can reach a paused run.
• It asks ExecuteWorkflowService whether the re-run request was sent rather than predicting it, so it never reports "re-running" over a toast saying the run was refused. A deployment can still reject the request after it leaves the browser; that arrives on the execution state stream.
• A refused write is reported with its real reason and keeps the suggestion on screen, instead of being reported as a failed analysis.

Gated on copilotEnabled, the flag the LiteLLM proxy already checks. The model is called only on an explicit click, never on render.

Screenshots

Recorded on Chrome at a 1536x639 viewport, against the current code. The workflow is TextInput → Python UDF → Filter → Sort; the UDF reads tuple_["lines"] while TextInput emits line. The JSON is in the reproduction comment below.

The recording runs the whole loop, untouched: Run starts the workflow, the traceback arrives while the execution is still Running, and the AI Fix tab appears beside Console on its own — the operator is already selected, so nothing but the console stream puts it there. Analyze returns the root cause and a diff with a confidence badge, quoting the whole line with its indentation. Apply and re-run stops the paused execution, writes the fix and re-runs; the pipeline then completes through all four operators and the tab removes itself, because the operator no longer reports an error.

The same panel when the error falls outside the four patterns -- here a ZeroDivisionError from the same operator. It is reported without calling the model at all, because the classification runs client-side before the request:

Any related issues, documentation, discussions?

Closes #8662

Hackathon submission for #8520 (Universidad Panamericana – Apache Texera Hackathon).

How was this PR tested?

• Unit tests: 297 across the result-panel specs — 63 for the service, 24 for the frame, 8 for the prompt builder, 68 for the result panel itself. Every fix in this PR was confirmed failing against the code before it, which caught six tests of mine that passed either way and had to be rewritten or deleted.

  cd frontend && yarn test --include="src/app/workspace/component/result-panel/**/*.spec.ts"
  

• Whole frontend suite: 6752 passing across 240 files (1 skipped). No substantive CI job has run on this branch — only the lightweight checks execute, the rest are still waiting on first-time-contributor approval — so everything CI runs on the frontend was run locally instead: yarn format:ci, yarn build:ci (clean production AOT build), yarn test:ci, and ng run gui:test-browser (23 passing in Chromium).

• End to end on a local stack (Windows, native services, LiteLLM + claude-haiku-4.5), as recorded above: the Python worker raised KeyError: 'lines' with the execution still Running; Analyze returned - title = tuple_["lines"] / + title = tuple_["line"] with high confidence, correctly citing the input schema; Apply patched the operator and the re-run reached Completed with rows through both downstream operators.

• The prompt's whole-lines rule was measured rather than assumed: 28 of 28 replies from claude-haiku-4.5 were accepted across the three code-change shapes, against 11 of 12 for the previous prompt, whose single refusal is exactly the mid-line snippet the rule exists for.

Was this PR authored or co-authored using generative AI tooling?

Yes. Generated-by: Claude Code (Claude Opus 5)

[github-actions]

👋 Thanks for your first contribution to Texera, @pedroreyesalavrado!

If you're looking for a good place to start, browse issues labeled starter-task; they're scoped to be approachable for newcomers.

You can drive common housekeeping yourself by commenting one of these commands on its own line:

• Issues. Comment /take to assign an open issue to yourself, or /untake to release it. You can find unclaimed work with the search filter is:issue is:open no:assignee.
• Sub-issues. To link issues into a parent/child hierarchy, comment /sub-issue #5166 #5222 on the parent to attach those children (or /unsub-issue #5166 #5222 to detach them). From a child issue, comment /parent-issue #5166 to set its parent, or /unparent-issue to clear it (the current parent is detected automatically). References may be written as #5166 or as a bare 5166; cross-repository references are not supported.
• Pull requests (author only). Comment /request-review @user to request a review from someone, or /unrequest-review @user to withdraw that request.

Each command must match exactly: /take this will not work, only /take does. For the full contribution flow, see CONTRIBUTING.md.

[github-actions]

Automated Reviewer Suggestions

Based on the git blame history of the changed files, we recommend the following reviewers:

• Contributors with relevant context: @yangzhang75, @eugenegujing, @kz930
  You can notify them by mentioning @yangzhang75, @eugenegujing, @kz930 in a comment.

[carloea2]

Initial review: please apply the stale-suggestion check to configuration changes as well as code changes.

In AiWorkflowFixerService.applyFix(), the code-change branch checks that the original snippet is still present, but the property-change branch unconditionally assigns properties[String(fix.fieldName)] = fix.suggested. If a user or collaborator changes the model setting after analysis, applying the old suggestion silently overwrites that newer value and reruns the workflow. Re-reading the properties preserves unrelated fields but does not protect the field being replaced.

Please compare the live field with the original value captured for the suggestion and require re-analysis when they differ. Add a regression test that changes the target property between analysis and Apply and verifies that neither setOperatorProperty nor executeWorkflow is called; retain coverage for an unchanged field applying successfully.

Based on static inspection of commit c286bcc; I have not run the tests.

[pedroreyesalvarado]

Good catch, thanks. Fixed in 0eb0a3f60.

applyFix() now applies the same staleness check to the property branch: it compares the live field with the value captured when the suggestion was generated, and refuses when they differ, so neither setOperatorProperty nor executeWorkflow runs and the panel asks for a fresh analysis. That matches what the code branch already did with its snippet.

Added the regression test you described — the target property changes between analysis and Apply, and both calls are asserted absent — and strengthened the existing unchanged-field test, which checked the write but not the re-run or the resulting state.

Tests pass locally on the rebased branch:

cd frontend && yarn test --include="src/app/workspace/component/result-panel/**/*.spec.ts"
Test Files  7 passed (7)
     Tests  201 passed (201)

[carloea2]

Thanks for the update, @pedroreyesalavrado. I inspected 0eb0a3f60: the live-value guard and the new test address the stale model-setting example from my review, and the unchanged-field test now checks the rerun. I have not independently run the tests.

Could you add side-by-side before/after screenshots and a short GIF or recording showing the error, Analyze, the suggested diff, Apply and re-run, and the resulting execution? Please also show Discard and an unsupported-error state. Include browser, viewport, commit, and reproduction steps using the supplied demo workflow.

Two tracking items: please restore the PR template's ### headings, and link a feature-specific issue for the AI fixer (or open one if none exists). #8520 is the general hackathon discussion: #8520.

Before merging, please also address the documented warehouse-selection case so the UI only reports "re-running" when execution actually starts.

[pedroreyesalvarado]

Thanks — the request for a recording paid for itself: setting it up surfaced two real bugs, both now fixed and pushed. Screenshots and the recording are in the PR description.

The tab did not appear in the common case. registerAutoRerenderResultPanel() re-rendered on execution-state, compilation, selection and result changes, but not on console messages. A Python UDF that raises while the user is already watching that operator moves none of those — the worker pauses so the execution stays Running, and the highlight never moves — so the tab only showed up if you clicked another operator and back. Fixed in 2f8bbb7 by adding the console stream to the merge, debounced 200ms since a chatty UDF's prints share it.

The tab lingered after the error cleared. It was set when the operator reported an error but never removed when it stopped, so a re-run left a tab offering to fix an error that was gone. Fixed in b8926d2, matching the teardown the Static Error frame beside it already had.

Both slipped through because my three existing tests called rerenderResultPanel() directly: they covered the branch but never what triggers it, which is exactly where the bugs were. The two new tests drive the stream instead.

Recording details: Chrome, viewport 1568x653, commit 56889cc, model claude-haiku-4.5 through the existing LiteLLM proxy.

Steps to reproduce (no dataset needed — the workflow sources from TextInput):

1. Bring up the stack with workflowWorkspace.copilotEnabled on and an LLM configured behind /api/chat/completions.
2. Import demo-workflows/pipeline_missing_column.json from the workflow list.
3. Select the Title length (missing column) operator, then Run. The Python worker raises KeyError: 'lines' and pauses; the execution stays Running.
4. The AI Fix tab appears beside Console. Click Analyze with AI.
5. The panel shows the root cause and the diff - tuple_["lines"] / + tuple_["line"]. Discard returns it to idle; Apply and re-run writes the fix and re-executes — Filter and Sort then produce rows, 8 → 8 → 6 → 6.

For the unsupported state in the second screenshot, edit that same UDF to raise something outside the four patterns (len(title) // 0) and click Analyze: the panel says so instead of guessing, and no request reaches the model.

The warehouse case you flagged is addressed in 19d21b6: when a warehouse is required and none is selected, Apply writes the fix but reports that separately instead of claiming a re-run ExecuteWorkflowService would refuse. Covered by three tests.

Full suite: 205 passing across the result-panel specs.

[Ma77Ball]

Review summary

The PR adds an AI Fix tab that classifies a failed Python-UDF error, asks the LiteLLM proxy for one minimal fix, and applies it through the undoable setOperatorProperty path. The core service and component are well structured and defensively coded: analysis fires only on an explicit click, apply re-reads the graph and guards against a stale suggestion on both the code and property branches, and the re-run is warehouse-aware. The specs are lean (small inline strings and a suggestion() helper), not JSON-heavy, so the mock test cases read fine. Two things should be settled before merge: the frame renders the shared singleton state without checking it belongs to the tab's own operator, so with two failed operators a suggestion can be shown and applied under the wrong one (P2); and the new top-level demo-workflows/ folder (README + 7 JSON files, ~800 lines referenced by nothing) is out of scope for the feature and, as it stands, its header-less README fails the license-header CI check (P1). No P0: the apply path is guarded and undoable, so nothing loses data.

Action items: Before merge: add the ASF header to demo-workflows/README.md or drop the folder, so check-header passes (P1); scope the AI Fix frame's state to its own operator so a suggestion cannot be applied to a different operator (P2); drop or relocate the demo-workflows/ folder (P2). The two P3s (over-broad 404 classification, hard-coded model alias) are optional.

---

Findings

5 total: 0 P0, 1 P1, 2 P2, 2 P3.

Blocking (P0/P1)

• Add the ASF license header to the new README (or drop the folder) - demo-workflows/README.md:1 - see inline.

Non-blocking (P2)

• Scope the AI Fix frame's state to its own operator - frontend/src/app/workspace/component/result-panel/ai-fix/ai-fix-frame.component.ts:72 - see inline.
• Drop or relocate the demo-workflows/ folder - demo-workflows/combined.json:1 - see inline.

Nit (P3)

• Tie the 404 match to the model-lookup context - frontend/src/app/workspace/component/result-panel/ai-fix/ai-workflow-fixer.service.ts:71 - see inline.
• Source the model id from config instead of a constant - frontend/src/app/workspace/component/result-panel/ai-fix/ai-workflow-fixer.service.ts:60 - see inline.

---

Verification

• P1 header: .github/workflows/check-header.yml runs apache/skywalking-eyes on every pull_request, driven by .licenserc.yaml. **/*.json is in paths-ignore (so the 7 JSON files are fine), but **/*.md is not, and every existing README.md in the repo carries the ASF <!-- ~ Licensed ... --> header. The new demo-workflows/README.md starts at # Demo workflows with no header, so that check fails on this PR.
• P2 cross-operator: ai-fix-frame.component.html binds state$ | async with no comparison to the frame's @Input() operatorId; AiWorkflowFixerService is providedIn: "root" with a single stateSubject; applyFix() (service:147) patches current.operatorId read from that shared state. Repro: operators A and B both raise an ERROR console message (both get the tab); analyze A, select B, B's tab shows A's diff, Apply patches A. Not covered by the specs (the "state survives re-subscription" test asserts the intended persistence, not a per-operator guard).
• P2 demo-workflows: git grep demo-workflows over the head tree returns no reference outside the folder; the README itself says 5 of the 7 need the external movies dataset, so they are demo aids, not fixtures.
• Not run: I did not execute the frontend specs or the live LLM/e2e path; correctness claims are from reading the changed files at 56889cc.

[Ma77Ball]

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.

[Ma77Ball]

Please drop this demo-workflows folder; it doubles the PR's line count, and I don't see any references to it. This also adds another level of complexity to maintaining the codebase and this feature.

[Ma77Ball]

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.

[Ma77Ball]

AI_FIXER_MODEL is hard-coded to the LiteLLM alias claude-haiku-4.5. I recommend sourcing the model ID from GuiConfig/env rather than using a compile-time constant.

[carloea2]

@aglinxinyuan Could you share your opinion on this UP hackathon proposal? Do you think the AI workflow fixer, which shows suggested changes for users to review before applying and re-running, is a direction we should pursue toward merging? Your input on the scope and integration with the existing workflow editor would help guide the next iteration alongside the review feedback already here.

[chenlica]

While we can use the agent to fix issues, it's very convenient to 1) highlight the places that have problems; and 2) have a button for the user to click to fix it.

It's good that this feature uses the existing LiteLLM proxy.

Overall, I like this new feature.

[aglinxinyuan]

This looks very polished. I agree with this change.

[pedroreyesalvarado]

Thanks @Ma77Ball — all four actionable findings are addressed in e749a0c and 931a977. The fifth I did not do, and I say why below rather than leave it looking handled.

P1, ASF header. You were right, and I verified it locally rather than from CI: of the 40 .md files tracked in the repo, 39 carry the header and the only one without it was mine. Worth noting the check never actually ran on this PR — the "Release Auditing" workflow is still pending maintainer approval for a first-time contributor, so only the seven lightweight checks executed. Your prediction just could not be observed yet.

P2, cross-operator state. A real bug, and the omission was mine: I put the state in a root singleton deliberately (the panel destroys and rebuilds every frame on re-render) but never scoped it to an operator. The frame now combines the service state with its bound operatorId and renders idle when they differ, so another operator's suggestion is neither shown nor applicable. Four tests cover it, including the sequence you described and the case where the bound id changes under a live suggestion.

P2, demo-workflows/ folder. Moved out of the repo root — but to bin/single-node/examples/ai-fix-demos/, beside examples/workflows/ rather than inside it, which is a deliberate departure from your suggestion. load-examples.sh iterates for workflow_file in "$WORKFLOW_DIR"/*.json, so putting them in workflows/ would create seven deliberately broken workflows in the account of anyone bringing the stack up with --with-examples. The glob is not recursive so a subdirectory would also work, but a sibling stays correct if it ever becomes recursive. The README explains this where the files live. Happy to drop the folder entirely if you would still rather not carry it.

P3, the bare 404. Fixed, and the false positive was real rather than theoretical — I added your cases as tests first and the suite went red on File "udf.py", line 404, in process_tuple classifying as model_not_found. The pattern now requires a model reference on the same line, which keeps the genuine shapes (ModelNotFound, 404 - model does not exist, and HuggingFace's 404 Client Error: Not Found for url: .../models/...) while dropping line numbers and unrelated HTTP statuses.

P3, model id from config — not done. GuiConfig has no model field today; I checked the 20 keys /api/config/gui actually returns and there is nothing to read it from. Adding one is a backend change, which this PR deliberately avoids (no Scala, no new endpoints). I have named the coupling in a comment at the constant so the next person hits it with context instead of a bare 404, and I am happy to do it as a follow-up PR that adds the config field properly. Let me know if you would rather it block this one.

Full suite: 209 passing. Thanks also @chenlica and @aglinxinyuan for weighing in on the direction.

[chenlica]

Hope this feature can be approved and merged into the main branch!

[pedroreyesalvarado]

@Ma77Ball gentle ping when you have a moment — the four actionable findings are addressed in e749a0c and 931a977, and two questions from my reply are still open:

• whether the demo folder should stay at bin/single-node/examples/ai-fix-demos/ or be dropped from this PR entirely;
• whether sourcing the model id from config should block this PR or go in a follow-up.

I am happy with either answer; I just do not want to guess and churn the branch.

One thing that may need a maintainer rather than me: the "Release Auditing" workflow has never run on this branch — only the seven lightweight checks execute, which I think is the first-time-contributor approval gate. That is probably why the PR reads as unstable. check-header has therefore never actually run here, so I verified it locally instead: every tracked .md file now carries the ASF header, including the one this PR adds.

[chenlica]

@kunwp1 Please review this PR.

[carloea2]

@kunwp1 Hi Chris, a quick reminder to revisit this PR when you have a chance. The author posted follow-up fixes and a response to your review on October 9, and the current head is e7c0d7b. Could you take another look and let us know whether your concerns are resolved or anything still blocks merging? Thanks!

[carloea2]

Thanks for the latest fixes. Could you check that suggestions and pending Apply operations are invalidated when the user switches workflows?

At e7c0d7b, the root service tracks the operator ID but not the workflow/session identity, and the workspace's Uninitialized reset does not clear the suggestion or supersede an in-flight analysis. In an isolated service reproduction using the PR source with mocked dependencies, I started an analysis in workflow A, emitted that reset, switched the graph to workflow B with the same operator ID, then completed the response. Apply wrote the suggestion to B and requested its execution.

This still needs browser verification; I have not confirmed the full navigation sequence end to end. Could you try two imports of the same workflow JSON and, if confirmed, bind analysis/Apply to the workflow session, invalidate them on a switch, and recheck the session after the asynchronous unlock wait? A regression test should verify that a response or Apply from A cannot modify or rerun B.

[carloea2]

Could you check the retry path after an Apply failure? At e7c0d7b, the panel keeps the suggestion visible in apply_failed and offers “Apply and re-run,” but applyFix() returns unless the status is ready.

I reproduced this with the actual service code and mocked dependencies: analyze a code fix, change the target snippet so Apply fails, restore the original snippet, then click Apply again. The second call silently returns without writing or rerunning.

Could you either accept apply_failed for a retry while rerunning all validation, or provide an explicit re-analysis action when the suggestion is stale? It would help to add a failure → correction → successful retry test so the button matches the behavior offered by the panel.

[carloea2]

Could you check structured configuration values in the property-fix path? The new field-name validation and number/boolean handling help, but at e7c0d7b an existing array or object field still falls through coerceToFieldType() and becomes a string.

In an isolated service reproduction using a mocked model response, fix_type: "property_change", original_snippet: "outputColumns", and suggested_snippet: "[]" replaced the operator's outputColumns array with the string "[]" and requested a rerun.

Could you restrict property fixes to supported fields/types, or parse and validate structured replacements against the operator's JSON schema before writing? Please cover an invalid array/object replacement leaving the graph unchanged and not rerunning, alongside a valid supported property fix.

[pedroreyesalvarado]

Thanks @carloea2 — all three are fixed in 55f5ccd and 88b22d9, each reproduced against the service before the fix and confirmed failing without it.

Suggestion bound to its workflow. You were right that the operator id alone does not identify the context, and the reproduction you described holds. FixState now carries the workflow it was made for, checked when the reply lands, before the write, and again after the unlock wait — that last one matters most, since the wait can take seconds and nothing stopped the user navigating during it.

Retry after a failed apply. This was the worst of the three and entirely my doing: I added apply_failed to the template two days earlier and left the guard in applyFix matching only ready, so the panel offered a button that silently did nothing. It is retryable now; every check below re-reads the operator and runs again, so a retry is not a shortcut past validation.

Structured configuration values. Arrays and objects are refused rather than parsed. Validating a replacement against the operator's JSON schema is a larger change than the four error shapes here need, and none of them touches a structured field, so refusing with a clear message beats guessing.

---

Your review also prompted me to audit what my own fixes touched, which turned up two more:

The frame still scoped by operator id alone. Refusing the write was only half of your first finding — the tab went on rendering a suggestion from another workflow, so the user saw a diff for a graph they had left and only learned otherwise by clicking Apply. Fixed in 88b22d9; the frame now renders nothing unless the state names both this operator and the open workflow.

An apply reported over whatever replaced it. During the unlock wait, any other failed operator's frame shows idle with a live Analyze button. Start an analysis there and the earlier apply announced "Fix applied" on top of it. The apply now reads the analysis counter when it begins and stays quiet if the panel has moved on.

One more worth mentioning because it explains a gap in what I could tell you before: the service spec had been injecting the real OperatorMetadataService, which fetches from its constructor, so every TestBed issued a live request. With enough tests the suite stopped reporting at all rather than failing — it was silently useless for a week. Stubbed now.

I also chased three further suspicions and they are all unreachable: the occurrence count going stale, the panel being destroyed mid-apply, and the operator being deleted during an analysis. All three are protected by the same thing — workflow modification is locked for as long as a run is live, which is exactly the window between analysing a traceback and applying the fix. I mention it in case it saves you reproducing them.

144 specs across the result panel, 6701 across the frontend, lint and production build clean. The branch is current with main.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

5 open issues

• applyFix() is async; calling it without await/void can violate common no-floating-promises… · New
• The docstring says “run actually started”, but the implementation returns true immediately after… · New
• The docstring says “run actually started”, but the implementation returns true immediately after… · New
• serif is a poor fallback for code/diff rendering and can change alignment; monospace is the… · New
• The comment is duplicated verbatim, which makes the block look like an accidental copy/paste.… · New

What changed in this PR

Adds an AI Fix tab to the Result Panel that can classify certain Python UDF failures, request a single minimal fix from an LLM, present it as a diff with confidence, and apply it back to the operator (undoable) with an optional re-run signal from ExecuteWorkflowService.

Changes:

• Add AiWorkflowFixerService + prompt builder + AI Fix frame UI, plus Result Panel wiring to show/hide the tab based on console ERROR messages while execution remains Running.
• Change ExecuteWorkflowService.executeWorkflow* entry points to return a boolean indicating whether local preconditions allowed the run request to be sent.
• Add extensive unit tests around classification, prompt redaction, diff display, apply semantics, and Result Panel tab toggling.

File	Description
frontend/​src/​app/​workspace/​service/​operator-menu/​operator-menu.service.spec.ts	Update mocks to match executeWorkflow() now returning boolean.
frontend/​src/​app/​workspace/​service/​execute-workflow/​execute-workflow.service.ts	Change executeWorkflow* to return boolean and document semantics.
frontend/​src/​app/​workspace/​service/​execute-workflow/​execute-workflow.service.spec.ts	Update delegate spy return type for the new boolean signature.
frontend/​src/​app/​workspace/​component/​result-panel/​result-panel.component.ts	Add AI Fix frame registration + console-driven toggling; fix display-name subscription accumulation.
frontend/​src/​app/​workspace/​component/​result-panel/​result-panel.component.spec.ts	Add coverage for console-driven AI Fix tab behavior and display-name handler wiring.
frontend/​src/​app/​workspace/​component/​result-panel/​ai-fix/​ai-workflow-fixer.service.ts	Implement classify/analyze/apply workflow with schema-aware secret stripping, staleness guards, and indentation-safe replacements.
frontend/​src/​app/​workspace/​component/​result-panel/​ai-fix/​ai-workflow-fixer.service.spec.ts	Add thorough unit tests for classifier, prompt safety, concurrency/staleness, apply behavior, and indentation logic.
frontend/​src/​app/​workspace/​component/​result-panel/​ai-fix/​ai-fix-prompt.ts	Build prompt and redact secrets; format multi-port schemas.
frontend/​src/​app/​workspace/​component/​result-panel/​ai-fix/​ai-fix-prompt.spec.ts	Add tests for redaction and schema formatting.
frontend/​src/​app/​workspace/​component/​result-panel/​ai-fix/​ai-fix-frame.component.ts	Add UI controller for Analyze/Apply/Discard and operator-scoped rendering.
frontend/​src/​app/​workspace/​component/​result-panel/​ai-fix/​ai-fix-frame.component.spec.ts	Add component tests for rendering, operator/workflow scoping, and button behavior.
frontend/​src/​app/​workspace/​component/​result-panel/​ai-fix/​ai-fix-frame.component.scss	Style diff/panel UI for the new tab.
frontend/​src/​app/​workspace/​component/​result-panel/​ai-fix/​ai-fix-frame.component.html	Add AI Fix tab UI (diff view, status alerts, confidence badge).
frontend/​src/​app/​workspace/​component/​menu/​menu.component.spec.ts	Update mocks for executeWorkflowWithEmailNotification() now returning boolean.

_{Review effort: Lite}

[pedroreyesalvarado]

@kunwp1 — all ten, plus the five from @Copilot. Three were in 7ee6339; the rest are in e8798c9 and e4e474f2.

Read-only workflows. You were right that this was worse than a missed check. enableWorkflowModification() is a no-op while the workflow is read-only, so the unlock the panel waits for never arrives — and running only needs write access to the computing unit, so a read-only user can reach a paused run. The old path killed their run on their behalf and then made them wait fifteen seconds to be told it would not stop. The lock is now refused up front unless a live execution is what holds it, since that is the only lock killing a run can clear.

Indentation was applied twice. A match that spans lines already carries the absolute indentation of the region it came from, and the model's reply mirrors it, so prefixing the context indentation on top doubled it. Only a single-line match needs the adjustment, which is the case the function was written for.

Zero occurrences. The panel only surfaces that count when it is above one, so a snippet the model invented was indistinguishable from any other suggestion — and Apply then turned it away as a stale snippet, blaming an edit that never happened. The analysis reports it now instead of offering an Apply that cannot work.

That one made me look at the rest of the apply path for the same shape, and there was a second instance. original for a property change is the field's current value, read so the diff has a "before" — so naming a field the operator does not have rendered an empty one, and naming code rendered the entire UDF source as one unwrapped line in a white-space: pre box. Apply refused both, but only after the panel had shown them. Both are rejected at the analysis now.

That also turned up a tautological test of mine. The apply-side field check had a test that passed with the check deleted, because the staleness guard covered the case on its own: an absent field reads as "" and no longer matches the captured original. With the field empty at the analysis both read "", and only the presence check can refuse — so it fails without it now. That is the sixth test of mine in this PR that passed either way; red-checking each one is the only reason I keep finding them.

code went to the model twice — fenced under "Operator code" and JSON-escaped inside the configuration, because code is a property like any other. The escaped copy is the one no snippet can be quoted from.

The state model. unsupported is a status now rather than a status plus an errorType, so the template reads one field to decide one thing and IDLE_STATE carries no placeholder classification. errorType stays, but only the apply path reads it, to refuse a configuration change for anything but model_not_found.

trackBy — you were right, and it undercut the change it was meant to support: keyvalue hands *ngFor a fresh pair object per entry, so every tab rebuilt whenever one was added, which is exactly the rerender the narrowed console subscription exists to avoid. The tabs are keyed by name now.

I should be straight about one thing: this one has no test. I tried to assert in the DOM that the tabs survive, but the loop only renders with the panel open, and that mounts the real frames — jsdom dies in HTMLCanvasElement.getContext. It is verified by reading, not by a test, and I would rather say so than let it look covered.

Comments that narrated history now state what the code does and which hazard the shape avoids, and the buildFixPrompt doc block is back on buildFixPrompt.

From @Copilot: onApply drops its promise explicitly; executeWorkflow's contract now says the request was sent rather than that the run started, which is the honest claim — a deployment can still reject it, and that arrives on the execution state stream; the diff falls back to monospace rather than serif, since the +/- columns stop lining up the moment monaco is missing; and the duplicated comment block is gone.

On the font: monaco, serif is the established idiom in this folder — console-frame and error-frame use it in seven other places. I only changed it in the diff, where the alignment actually matters, rather than touching files this PR has no business in. Happy to open a separate one for the rest if you want it.

Also worth flagging: Copilot's review did not finish — "couldn't run its full agentic review because it didn't start before the timeout", then "the job was not acquired by a runner even after multiple attempts". So those five are a partial pass, not a clean one. Re-request it if you want the full sweep; I would rather it find something now than after a merge.

Frontend suite: 6745 passing, 290 of them across the result-panel specs (57 service, 24 frame, 8 prompt builder, 67 result panel). eslint ./src, prettier --check and tsc --noEmit clean.

[kunwp1]

Thanks for working through the last round. I re-checked those comments and most are resolved. Three areas aren't fully there yet and they're the comments below.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 92.14502% with 26 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.33%. Comparing base (3ba45ee) to head (b3f3606).
⚠️ Report is 32 commits behind head on main.

Files with missing lines	Patch %	Lines
...t/result-panel/ai-fix/ai-workflow-fixer.service.ts	88.64%	11 Missing and 10 partials ⚠️
...nent/result-panel/ai-fix/ai-fix-frame.component.ts	92.68%	1 Missing and 2 partials ⚠️
...nt/result-panel/ai-fix/ai-fix-frame.component.html	98.24%	0 Missing and 1 partial ⚠️
...e/component/result-panel/result-panel.component.ts	96.55%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8637      +/-   ##
============================================
- Coverage     92.33%   92.33%   -0.01%     
  Complexity     5045     5045              
============================================
  Files          1261     1265       +4     
  Lines         54120    54440     +320     
  Branches       6768     6838      +70     
============================================
+ Hits          49973    50266     +293     
- Misses         2486     2498      +12     
- Partials       1661     1676      +15     

Flag	Coverage Δ		*Carryforward flag
access-control-service	77.38% <ø> (ø)		Carriedforward from 711b328
agent-service	99.16% <ø> (ø)		Carriedforward from 711b328
amber	87.31% <ø> (ø)		Carriedforward from 711b328
computing-unit-managing-service	60.48% <ø> (ø)		Carriedforward from 711b328
config-service	87.37% <ø> (ø)		Carriedforward from 711b328
file-service	82.00% <ø> (ø)		Carriedforward from 711b328
frontend	96.49% <92.14%> (-0.07%)	⬇️
notebook-migration-service	83.73% <ø> (ø)		Carriedforward from 711b328
pyamber	98.57% <ø> (ø)		Carriedforward from 711b328
workflow-compiling-service	72.43% <ø> (ø)		Carriedforward from 711b328

*This pull request uses carry forward flags. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pedroreyesalvarado]

@kunwp1 — all four, one commit each: 68c65b1, 03cd234, 114b582, e35d097.

The JVM one was a regression of my own fix, not a gap in it. You were right about the module prefix, and I checked FRAME_LINE against six real frame shapes before touching anything: java.base/java.lang.Thread.run(Thread.java:840), the ThreadPoolExecutor frame, (Native Method) and <init> all fall through — only the two shapes I had imagined match. And mkConsoleMessage sends getStackTrace.mkString with no at prefix, so the one pattern I added for the JVM never applied to console messages at all. A real worker trace ends on exactly one of those frames, so the backward scan returned the frame and NullPointerException classified as unsupported. What I shipped in 7ee6339 did not work in production.

So I took your suggestion rather than patching the regex. I checked all three sources first: mkConsoleMessage puts err.toString in title, the Python worker puts format_exception[-1] there, and a fatal error carries evt.e.toString in message with the stack in details. analyzeError now takes both parts — header to classify, full for the prompt — and FRAME_LINE and finalExceptionLine are gone. That retires the chained-traceback handling too, instead of reimplementing it: format_exception[-1] is already the exception that stopped the worker, which is what scanning backwards was trying to recover.

Validating after the kill was the worst of the four in practice, and the test makes the cost visible: it took 15 seconds to fail. The run was already stopped, and then the user waited out the unlock timeout to be told nothing had been written. The checks and the patch moved into patchedProperties(), which reads only the graph, so it runs as a pre-flight before the kill and again after it — the operator can still move during the wait. The model_not_found check moved to the analysis, where the rest of them already were.

On the indentation, you were right that the prompt never stated a convention, so I stated one and measured it rather than guessing. The prompt now requires whole lines copied from the start of a line with their indentation; startsAtLineBoundary refuses a reply that quotes mid-line, checking every occurrence rather than the first, since one mid-line match is enough to make the result depend on a guess. replaceAllIndented is then just a replacement, and is named replaceAll.

I ran it against claude-haiku-4.5 through the local LiteLLM proxy on the three code-change shapes, with the previous prompt as a control:

	accepted
new prompt	28 / 28
old prompt (control)	11 / 12

The single refusal under the old prompt is exactly a mid-line snippet with no indentation — the case this is about. So the model was usually including the indentation on its own, which is why this looked fine end to end and still broke the way you found. Applying the model's real multi-line guard now yields 0/4/8/12 nesting, valid Python, nothing doubled.

The test fixture had to change with it, which says the same thing from the other side: its original_snippet was quoted mid-line, so the new contract turns the old fixture away.

trackBy — keying by the config object, as you suggested. It also settles what I admitted I could not test: "toggles only the AI Fix entry on a console update" already asserts that the Console config object survives by identity, which is exactly what the trackBy now keys on, so the claim is no longer resting on my reading of NgComponentOutlet.

One more that nobody reported. Reviewing the branch myself I found that Apply checked only whether the snippet was still present, never whether it was still in the same number of places — so a copy added between the analysis and the click was patched too, with the "changes N places" badge hidden at one occurrence. It refuses in both directions now (711b328).

Suite: 6751 passing, eslint ./src, prettier --check and tsc --noEmit clean.

One ask. The approved re-run on 711b328 passed everything (Required Checks was cancelled by my next push, not failed), but every push since is back to action_required, so none of these four fixes has actually been through CI. Could you approve a run when you next look? Four changes went in off the back of unit tests alone, and the classification one touches a path I got wrong once already — I would rather CI contradict me now than after a merge. The same goes for Copilot, whose review ran out of runners last time.

[kunwp1]

Thanks for the quick turnaround. All four comments from the last round look resolved.

The remaining comments are mostly about which errors can actually be fixed end to end.

[kunwp1]

This checks that each match starts a line but not that it ends one. So a whole line can still match the beginning of a longer line. For example, an original of n = 1 in code containing n = 10 passes the check with a count of 1 so no badge appears. "Apply" then turns that line into n = 20 while the diff shows only n = 1. Requiring each match to also end at a newline (or the end of the code) could fix this issue.

[kunwp1]

I couldn't find an operator where two of these rows can finish end to end:

• model_not_found. The HuggingFace operator never raises for an unknown model. It writes "All inference providers failed" into its result rows, so there's no ERROR message and no tab. A malformed id raises ValueError: Invalid Hugging Face model ID ..., which none of these patterns match. In a Python UDF the model name lives in code, but the prompt tells the model to change a configuration field, which the new analysis check then refuses.
• NullPointerException. It now classifies, but a JVM operator other than a UDF has no code and no allowed configuration change, so Analyze can only end in an error after a model call.

Could the supported list (and when the tab is offered) be narrowed to what can actually be fixed today? Alternatively, the model rule could allow a code change when the operator has code.

[kunwp1]

Now that analysis refuses a configuration change for anything but model_not_found, this check can't pass.