You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@rocketmq.apache.org by "Loyal-Young (via GitHub)" <gi...@apache.org> on 2026/10/09 08:28:38 UTC
[PR] fix(ai): invalidate runtime loads when the page unmounts [rocketmq-dashboard]
Loyal-Young opened a new pull request, #5779:
URL: https://github.com/apache/rocketmq-dashboard/pull/5779
### Which Issue(s) This PR Fixes
- Fixes #5777
### Brief Description
Invalidate the LLM runtime request generation when its effect is cleaned up. A configuration response arriving after leaving the AI page could still invoke the callback that writes the global persisted engine preference and start a model lookup. Late configuration/model failures could also show an obsolete error toast.
The existing generation guard now also covers unmounting.
### How Did You Test This Change?
- On the original code, all three new deferred regressions fail: the engine callback and both error callbacks fire after unmount.
- `cd web; node node_modules/vitest/vitest.mjs run src/pages/ai/hooks/useLlmRuntime.test.ts --pool=threads --maxWorkers=1 --reporter=dot` — 6 tests passed.
- `cd web; node node_modules/typescript/bin/tsc -p tsconfig.app.json --noEmit --pretty false` — passed.
- `cd web; node node_modules/eslint/bin/eslint.js src/pages/ai/hooks/useLlmRuntime.ts src/pages/ai/hooks/useLlmRuntime.test.ts` — passed.
- `git diff --check` — passed.
- Tested against `rocketmq-studio@c99b9ad50b79467546997e51f3401d57ceebea5f`. Full application build and backend tests were not rerun for this hook-only change.
### Checklist
- [x] One coherent change; no unrelated modifications
- [x] Conventional commit title
- [x] Regression tests for the changed behavior
- No new UI text, source files, dependencies or backend changes.
### AI Assistance
AI assistance was used to inspect the asynchronous lifecycle, draft the patch and deferred regression cases, and run the verification commands. The regression cases were run against the original implementation before applying the fix.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(ai): invalidate runtime loads when the page unmounts [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins commented on PR #5779:
URL: https://github.com/apache/rocketmq-dashboard/pull/5779#issuecomment-6093315220
> Thanks — the defect is real and the fix is the right one: after unmount `optionsRef.current` still holds the last render's callbacks, so `useLlmRuntime.ts:108` writes the globally persisted engine preference and line 113 fires an extra `getLlmModels()` request. Reusing the existing `loadSeqRef` generation guard in the effect cleanup is minimal and correct, and the three deferred regressions are mutation-sensitive. > > The blocker is not technical: #5782 fixes the same issue (#5777) and the two branches genuinely conflict in `web/src/pages/ai/hooks/useLlmRuntime.test.ts`. #5782 also adds a page-level regression in `AiPage.test.tsx` that pins the persisted-store overwrite itself. We will pick one of the two and close the other with a pointer — no action needed from you until then. > > Optional follow-up (not required for this PR): 15 files use the same `requestId = useRef(0)` generation pattern and only `useConversationTimeline.ts` has an unmount cleanup. The rest only call `s
etState` after unmount, which React 18 makes harmless, but `useConversationList.ts` and the `onError` call sites in `useAgentRun.ts` (lines 275/356/408) are worth a separate pass.
---
**Decision**: we are taking #5782 for this defect - it covers the unmount dimension on a newer baseline and its cases include the StrictMode and integration-level paths. Closing this one as a duplicate. Thanks for finding it independently; the fix lands through #5782 and your analysis above is recorded there.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(ai): invalidate runtime loads when the page unmounts [rocketmq-dashboard]
Posted by "lizhimins (via GitHub)" <gi...@apache.org>.
lizhimins closed pull request #5779: fix(ai): invalidate runtime loads when the page unmounts
URL: https://github.com/apache/rocketmq-dashboard/pull/5779
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] fix(ai): invalidate runtime loads when the page unmounts [rocketmq-dashboard]
Posted by "Loyal-Young (via GitHub)" <gi...@apache.org>.
Loyal-Young commented on PR #5779:
URL: https://github.com/apache/rocketmq-dashboard/pull/5779#issuecomment-6094265928
Thanks for verifying the side effect and recording the analysis with #5782. I agree with selecting the newer branch with StrictMode and page-level persisted-store coverage. I will leave this duplicate closed. For any follow-up on the suggested hooks, I will first verify an observable external side effect and check existing reports, rather than adding blanket guards around harmless post-unmount state updates.
--
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: commits-unsubscribe@rocketmq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org