pedroreyesalavrado commented on PR #8637: URL: https://github.com/apache/texera/pull/8637#issuecomment-6047019638
Thanks for the approval and for the follow-ups — all four are in c6157cb, plus two more I found while auditing, in 39185db. **Duplicate registration in `displayError`.** Removed, and your read was right on both halves. With an operator selected, the console-error block that runs later in the same method deletes it again, so it was dead. With no operator — a whole-workflow failure calling `displayError(undefined)` — that block does not run, so the tab survived with an undefined operator id and a permanently disabled Analyze button. No test depended on it. **Frozen JWT.** A real bug, and mine: memoizing the provider pinned the token captured on the first Analyze, so a later call after a rotation would 401 and surface as the generic "could not suggest a fix". The provider is now built per call. I want to be explicit that this one ships **without a unit test**. The specs stub the transport at the `callModel` seam precisely to keep the `ai` module out of the test process. Any test for this has to reach past that seam and let `generateText` hit the wire. I wrote one, found it passed with the bug reintroduced, and deleted it rather than leave something that looks like cover and is not. If you know a way to assert it that fits the file's convention, I will take it. **`replace` patching only the first occurrence.** Fixed with `split`/`join` as you suggested. This one does have a test, and I confirmed it fails against the old code first: a UDF reading the same wrong column twice came back half-fixed and failed the re-run on the second read. **`ai-fix-demos/` folder.** Removed — 849 lines out of the diff. The reproduction JSON is in a separate comment below so the recording stays reproducible without the repo carrying it. Two more that came out of auditing the above: **A race between two analyses.** Scoping each frame to its own operator (your earlier P2) left the other frames rendering idle, which keeps their Analyze button live while one operator is mid-analysis. Two requests can then be in flight over one state subject, and a slow first reply overwrote a newer one — the user asked for an analysis on the second operator, waited, and got nothing. Analyses are numbered now and a superseded reply is dropped. Test confirmed failing first. **A lint error that would have gone red.** `KEY_ERROR` in the service spec was a template literal with no interpolation, which the repo's `quotes` rule rejects. Worth flagging why nobody caught it: **no substantive CI job has ever run on this branch.** Only seven lightweight checks execute; lint, the test job and "Release Auditing" are all still waiting on first-time-contributor approval. Everything I report here is verified locally — 211 specs, `eslint ./src` clean, and a clean production build — but a maintainer approving the workflows may well surface something none of us has seen yet. -- 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]
