Frun1na opened a new pull request, #6796:
URL: https://github.com/apache/rocketmq-dashboard/pull/6796
### Which Issue(s) This PR Fixes
- Fixes #6794
### Brief Description
`rmq_ai_conversation.model` and `rmq_ai_run.model` are `VARCHAR(128)`
(`schema.sql:459`, `:479`) and
store the configured LLM model verbatim — `AiConversationService.create`
sets it from the stored
config (`AiConversationService.java:175`) and the run row snapshots the
resolved id
(`AiRunService.java:182`, `:473`) — while neither configuration endpoint
bounds it:
`GeneralSettingsUpdateDTO.model` carried only `@NotBlank` (`:55`) and
`LlmConfigService.validate`
checks the API base, `maxTokens` and the temperature but not the model
(`:181-213`); the
`/api/llm/config` body is not bean-validated at all
(`LlmController.java:40-41`). A model id of 129
characters or more therefore saved with a 200 and then made the
conversation's first request fail
inside the insert, where MySQL rejects the row and the generic handler
answers 500
(`GlobalExceptionHandler.java:108-112`). The AI settings page offers
whatever model ids the
configured gateway reports, so such an id is selectable in the console.
Bound the id to the column width at the four places that matter: the
settings DTO and the LLM
config validation reject it with a 400 naming the field, and the
conversation create and run
admission refuse an over-long *stored* value the same way, so an existing
bad configuration no
longer turns into a 500 either. The per-run override was already bounded to
the same 128 characters
by `AiMessageDTO.model:45`, so this restores one rule for one field.
User-visible change: storing a model id longer than 128 characters answers
`400` with a message
instead of `200`, and the first message of a conversation whose *stored*
model is too long answers
`400` instead of `500`.
### How Did You Test This Change?
```
cd server && mvn -B -ntp test -DforkCount=1 \
-Dtest=LlmConfigServiceTest,AiConversationServiceTest,AiRunServiceTest,GeneralSettingsModelBoundTest
Tests run: 32, Failures: 0, Errors: 0, Skipped: 0 -- LlmConfigServiceTest
Tests run: 28, Failures: 0, Errors: 0, Skipped: 0 -- AiRunServiceTest
Tests run: 22, Failures: 0, Errors: 0, Skipped: 0 --
AiConversationServiceTest
Tests run: 1, Failures: 0, Errors: 0, Skipped: 0 --
GeneralSettingsModelBoundTest
Tests run: 83, Failures: 0, Errors: 0, Skipped: 0 (checkstyle:check green
in the same run)
```
The new tests fail without the fix — one per guarded surface, which is also
what shows the four
guards are each observable:
```
Tests run: 83, Failures: 4, Errors: 0, Skipped: 0
LlmConfigServiceTest.saveConfigShouldRejectAModelLongerThanTheAiRecordColumnsTest
Expecting code to raise a throwable.
AiConversationServiceTest.createShouldRefuseAConfiguredModelTheConversationColumnCannotStoreTest
Expecting code to raise a throwable.
AiRunServiceTest.admissionShouldRefuseAConfiguredModelTheRunColumnCannotStoreTest
Expecting code to raise a throwable.
GeneralSettingsModelBoundTest.modelShouldBeBoundedToTheAiRecordColumnsTest
Expecting any elements of: [] to match given predicate but none did.
```
### Other consumers
The model id reaches four write/read paths; each one is covered or explained:
| Surface | Status |
| --- | --- |
| `POST /api/settings/general/save` (`GeneralSettingsUpdateDTO.model`) |
fixed: `@Size(max = 128)` (bean validation runs, the controller is `@Valid`) |
| `POST /api/llm/config` (`LlmConfigService.validate`) | fixed: rejected
with `400 llm.config.invalid_model`, next to the existing
`maxTokens`/temperature checks |
| `AiConversationService.create` (conversation row) | fixed: a stored
over-long id is refused with a 400 before the insert |
| `AiRunService` admission (run row) | fixed: the resolved id is checked
after `resolveModel`, covering the configured model and the conversation's |
| Per-run override (`AiMessageDTO.model`) | already correct: bounded to 128
by `@Size` |
| The provider call itself | unchanged: a model that fits the records still
reaches the gateway verbatim |
### Checklist
- [x] One coherent change; unrelated modifications are not bundled in
- [x] Commit subject follows Conventional Commits (`feat:` / `fix:` /
`refactor:` / `chore:` / `docs:` / `perf:`)
- [x] Tests added or updated for non-trivial changes, test methods named
`...Test`
- [ ] New UI text has both Chinese and English entries under `web/src/i18n/`
- [ ] Architecture constraints stay green (`mvn test` runs the ArchUnit
checks)
- [x] New source files carry the ASF license header
- [ ] Documentation touched where behaviour changed (README / `docs/` /
in-app help)
--
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]