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]

Reply via email to