[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 three 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	.dropna() or a null guard

Python UDFs only, and one kind of fix: a change to code. An operator without code would cost a model call to reach a refusal, so the tab is not offered there — a JVM operator raising NullPointerException classifies, but has nothing Apply could patch. A configuration fix is not offered either: nothing in the repo raises an error one could repair, since the HuggingFace operator retries a 404 as a chat completion and writes "All inference providers failed" into its result rows rather than raising.

Classification reads the exception header, which both sources already separate from the stack: mkConsoleMessage puts err.toString in title, the Python worker puts format_exception[-1] there, and a fatal error carries it in message with the frames in details. Nothing has to find the exception inside the joined text, which also means a chained traceback arrives already reduced to the one that stopped the worker.

What leaves the browser. The error text, the operator's code and every input port's schema. Not the operator's configuration: no fix this panel writes reads it, and filtering it could never be made safe. withoutSecrets matched top-level field names, but a Python UDF's uiParameters is a list of attribute/value pairs, so the name says nothing about whether a value is an API key the user typed in — and the list went into the prompt whole. The block is gone rather than filtered harder, which takes the two sanitisers, the operator's JSON schema lookup and the OperatorMetadataService dependency with it.

The model the panel asks for is the alias the deployment's LiteLLM proxy publishes, read from GuiConfig (gui.workflow-workspace.ai-fix-model, overridable by GUI_WORKFLOW_WORKSPACE_AI_FIX_MODEL), so a proxy serving a different alias does not need a rebuild to be reachable.

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. That decision is made against the frame map itself rather than anything remembered between emissions, because rerenderResultPanel writes to the same map: a remembered key went stale the moment the rerender dropped the tab, and the next traceback was then read as "no change". The frame tabs are keyed by their config object, since keyvalue hands *ngFor a fresh pair per entry and NgComponentOutlet only recreates a component when its type changes. 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 has moved on since the suggestion was generated.
• A suggestion that cannot be applied never reaches the screen. Refused at the analysis, not at the click: a snippet the model invented (zero matches), one quoted from the middle of a line, one that begins a line but stops inside a longer one, and any reply that is not a code change. A quote may include its line's trailing newline or not — one is dropped from each snippet, so the verdict does not depend on where in the file the line sits.
• 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, and Apply refuses if the count has changed since — an edit in between would otherwise touch a different number of lines than the panel promised.
• No indentation is inferred. The prompt requires whole lines carrying the indentation they should have, and a reply that does not is refused, so the replacement is a replacement and nothing has to guess which convention the model used.
• 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. Every other refusal is checked before that kill as well as after it, so a fix that was never going to be written does not cost the user their 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 three 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: 295 across the result-panel specs — 57 for the service, 23 for the frame, 4 for the prompt builder, 69 for the result panel itself. Every fix in this PR was confirmed failing against the code before it, which caught eight 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: 7011 passing across 247 files (1 skipped). Everything CI runs on the frontend was run locally too: 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, @mengw15, @eugenegujing
  You can notify them by mentioning @yangzhang75, @mengw15, @eugenegujing 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.

[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.

[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 91.83673% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.96%. Comparing base (8f30d82) to head (7d3ccda).
⚠️ Report is 5 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
...t/result-panel/ai-fix/ai-workflow-fixer.service.ts	88.07%	9 Missing and 9 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.14%	0 Missing and 1 partial ⚠️
...ace/component/result-panel/ai-fix/ai-fix-prompt.ts	85.71%	0 Missing and 1 partial ⚠️
...e/component/result-panel/result-panel.component.ts	96.96%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #8637      +/-   ##
============================================
- Coverage     91.96%   91.96%   -0.01%     
  Complexity     5084     5084              
============================================
  Files          1280     1284       +4     
  Lines         55682    55965     +283     
  Branches       7063     7121      +58     
============================================
+ Hits          51208    51468     +260     
- Misses         2732     2742      +10     
- Partials       1742     1755      +13     

Flag	Coverage Δ		*Carryforward flag
access-control-service	77.44% <ø> (ø)
agent-service	99.16% <ø> (ø)		Carriedforward from 8f30d82
amber	86.38% <100.00%> (+<0.01%)	⬆️
computing-unit-managing-service	61.81% <ø> (ø)
config-service	87.61% <100.00%> (+0.11%)	⬆️
file-service	82.29% <ø> (ø)
frontend	96.47% <91.75%> (-0.06%)	⬇️
notebook-migration-service	83.77% <ø> (ø)
pyamber	98.54% <ø> (ø)		Carriedforward from 8f30d82
workflow-compiling-service	74.25% <ø> (ø)

*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.

[pedroreyesalvarado]

@kunwp1 — all three, in 10d377589 and 0935e669e, on top of a merge with main.

The mid-line match was a real one and I had reasoned my way past it. When I made the prompt require whole lines I argued that checking the start was enough, because that is what makes the indentation unambiguous. It is not enough for the replacement: n = 1 matches n = 10 once, so the count is 1 and the occurrence badge stays hidden, and Apply leaves the trailing 0 behind. The diff offers n = 1 → n = 2 and the operator is left saying n = 20. Both ends are checked now and the function is called matchesWholeLines.

The errorType check was unreachable, and you are right that it is not like the field checks. I had kept it as a second barrier by analogy with those, but a field can disappear between the analysis and the write, while errorType is fixed when the state is published and the analysis already refuses a property fix for anything else. It is gone.

On what can actually be fixed end to end, I checked the repo and it is worse than your two cases. model_not_found has no reachable producer at all: a 404 from the HuggingFace inference API is retried as a chat completion, and when every provider fails the text goes into the result rows through _format_error rather than being raised — no exception, no ERROR console message, no tab. The malformed-id path does raise, but as ValueError: Invalid Hugging Face model ID ..., which the pattern never matched. The sklearn operators have a model field and no remote lookup to 404 on.

So I took the narrowing. Three shapes, Python UDFs only, one kind of fix. NullPointerException goes with it for the reason you gave: a JVM operator has no code for Apply to patch.

That left the configuration path with no way in, and dead code is what your previous comment was about, so it went too: the property branch of patchedProperties, coerceToFieldType, type and fieldName on SuggestedFix, errorType on FixState (that branch was its only reader), the field row in the diff, and the model rule in the prompt. Fourteen test blocks went with it — 132 lines added, 346 removed.

One thing I want to flag rather than let it pass. Removing model_not_found turned my credential test into a test of nothing: it analysed a model error, which is now unsupported, so the model was never called and the assertion had no prompt to inspect. It would have stayed green. It asks for a supported error now, so it still proves what it claims — that the service redacts before building the prompt. That is the seventh test of mine in this PR that passed either way.

The PR body is updated, including three claims that had gone stale earlier and that I should have caught: it still described the final-exception-line heuristic, the tabs keyed by name, and the indentation inference, all of which are gone.

Suite: 6959 passing. yarn format:ci, yarn build:ci, yarn test:ci and ng run gui:test-browser all clean locally — still worth a CI run when you get a chance to approve one, since none of these commits has been through it.

[kunwp1]

LGTM! The rest are the small follow-ups.

[pedroreyesalvarado]

@kunwp1 — all three follow-ups, plus @Ma77Ball's model-id comment from the first round. d4b8068e and 7d3ccda5, on top of a merge with main.

The uiParameters leak was real and I reproduced it before touching anything: an hf_live_... value passed as a UI parameter appears verbatim in the built prompt. Your diagnosis of why filtering harder would not help is exactly right — the field name says nothing about what a list of attribute/value pairs holds.

So the configuration block is gone rather than better filtered, and with it withoutSecrets, withoutCode, schemaProperties and the OperatorMetadataService dependency. analyzeError no longer takes the properties either. The PR body claimed credentials were kept out behind two independent barriers; that claim is retired, because now there is nothing to keep out.

On the trailing newline, I had reasoned my way into exactly the mirror of the bug I fixed yesterday. I argued that checking the start of the match was enough, then added the end check for the n = 1 / n = 10 case without noticing it made the verdict depend on where the line sat. One trailing newline comes off each snippet now, independently, so the replacement leaves the line's own newline in place. The prompt says the newline is optional instead of leaving it to chance.

Worth recording how I got there, since it took two tries: my first test called matchesWholeLines with a snippet that was not in the code at all, so it returned true vacuously and proved nothing. The second quoted the last line of the file — the one case that already worked, which is what hid this in the first place. Only a line in the middle turns it red.

fix_type is gone. With one legal value it could only refuse a good answer. The test that used to rely on it now says what actually turns a field-name reply away: the occurrence count.

And @Ma77Ball's point from the first round is addressed too. AI_FIXER_MODEL was a compile-time constant holding the alias this deployment's proxy publishes. It now travels the same route as every other workspace setting — gui.workflow-workspace.ai-fix-model, overridable by env, served beside copilotEnabled. Deliberately not in llm.conf, which the frontend never sees: the panel names the model, while the proxy's base URL and master key stay backend-only.

I deleted the unit test I wrote for that wiring rather than keep it green. It asserted the value I had just set and that callModel was called — true whether or not the id reaches .chat(), since callModel is the seam every other test stubs. Verified against the running stack instead: the config endpoint serves aiFixModel, the browser receives it, and the served bundle calls .chat(this.config.env.aiFixModel) with no trace of the constant. That is the eighth test of mine in this PR that passed either way.

Suite: 7011 passing. yarn format:ci, yarn build:ci, yarn test:ci and ng run gui:test-browser all clean locally.

One ask. Every push puts the workflows back behind first-time-contributor approval, so none of these commits has been through CI — the run that passed was on 0935e669e. Could you approve one when you next look? The config change touches GuiConfig and ConfigResource, so it is the first commit here that CI would exercise beyond the frontend.

[kunwp1]

@carloea2 Do you have any other comments for this PR?

[github-actions]

⚠️ Benchmark changes need a look

🟢 2 better · 🔴 7 worse · ⚪ 6 noise (<±5%) · 0 without baseline

Compared against main 8f30d82 benchmarked on this same runner, so the delta is largely free of cross-runner hardware noise. The "7d avg" column still reflects the gh-pages dashboard. Treat <±5% as noise unless repeated.

Dashboard · Run

	config	throughput	MB/s	latency	max Δ latest / 7d
🔴	bs=10 sw=10 sl=64	404	0.247	23,997/31,115/31,115 us	🟢 -24.2% / 🔴 +125.9%
🔴	bs=100 sw=10 sl=64	851	0.519	115,262/133,608/133,608 us	🔴 +11.1% / 🔴 +43.5%
🔴	bs=1000 sw=10 sl=64	967	0.59	1,017,263/1,161,385/1,161,385 us	🔴 +6.0% / 🔴 +31.8%

Baseline details

Latest main 8f30d82 from same runner

config	metric	PR	latest main	7d avg	Δ latest	Δ 7d
bs=10 sw=10 sl=64	throughput	404 tuples/sec	402 tuples/sec	941.58 tuples/sec	+0.5%	-57.1%
bs=10 sw=10 sl=64	MB/s	0.247 MB/s	0.246 MB/s	0.575 MB/s	+0.4%	-57.0%
bs=10 sw=10 sl=64	p50	23,997 us	21,401 us	10,991 us	+12.1%	+118.3%
bs=10 sw=10 sl=64	p95	31,115 us	41,063 us	13,776 us	-24.2%	+125.9%
bs=10 sw=10 sl=64	p99	31,115 us	41,063 us	16,872 us	-24.2%	+84.4%
bs=100 sw=10 sl=64	throughput	851 tuples/sec	899 tuples/sec	1,264 tuples/sec	-5.3%	-32.7%
bs=100 sw=10 sl=64	MB/s	0.519 MB/s	0.549 MB/s	0.772 MB/s	-5.5%	-32.7%
bs=100 sw=10 sl=64	p50	115,262 us	109,959 us	86,073 us	+4.8%	+33.9%
bs=100 sw=10 sl=64	p95	133,608 us	120,264 us	93,135 us	+11.1%	+43.5%
bs=100 sw=10 sl=64	p99	133,608 us	120,264 us	111,124 us	+11.1%	+20.2%
bs=1000 sw=10 sl=64	throughput	967 tuples/sec	984 tuples/sec	1,304 tuples/sec	-1.7%	-25.9%
bs=1000 sw=10 sl=64	MB/s	0.59 MB/s	0.601 MB/s	0.796 MB/s	-1.8%	-25.9%
bs=1000 sw=10 sl=64	p50	1,017,263 us	1,013,672 us	841,986 us	+0.4%	+20.8%
bs=1000 sw=10 sl=64	p95	1,161,385 us	1,095,955 us	881,107 us	+6.0%	+31.8%
bs=1000 sw=10 sl=64	p99	1,161,385 us	1,095,955 us	918,208 us	+6.0%	+26.5%

Raw CSV

config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,494.70,200,128000,404,0.247,23996.87,31115.31,31115.31
1,100,10,64,20,2350.32,2000,1280000,851,0.519,115262.35,133607.88,133607.88
2,1000,10,64,20,20683.24,20000,12800000,967,0.590,1017262.62,1161384.90,1161384.90

[pedroreyesalvarado]

The two failing legs are flaky tests, and neither is in this PR's diff:

• ubuntu — migration-llm.spec.ts > falls back to the default timeout when the configured value is non-positive, failing with TypeError: this.validationWorkflowService.getValidTexeraGraph is not a function from a debounceTime emission in WorkflowCompilingService. A debounced subscription fires after the spec's injector is gone.
• macOS — dataset-selection-modal.component.spec.ts > onDatasetChange does nothing while no dataset is selected, failing with Test timed out in 20000ms.

Different tests on different runners, both outside this PR, is the shape of flakiness rather than a regression — a regression fails in the same place everywhere. Windows passed. It also matches what vitest.config.ts already documents about ubuntu and macOS wall-time swings (#7713), and the macOS one is that 20s ceiling being hit.

Locally I ran everything CI runs on the frontend — format:ci, build:ci, test:ci and ng run gui:test-browser — all clean, though on Windows, which is exactly the leg that passed here.

Could you re-run the two failed jobs? I do not have permission to. If they fail on the same two tests again, that is worth raising with the owners of those specs rather than holding this PR.

[carloea2]

Reviewed the latest changes at 7d3ccda. The earlier concerns are addressed through whole-line code replacements, removal of configuration edits and configuration data from the prompt, read-only/live-execution guards, and stable frame tracking. I also checked the model configuration wiring added after Chris's approval. Required CI checks are passing. No additional blocking comments from me.