fix(chat): send temperature through modelOptions, where LLPhant reads it - #3243
Merged
rubenvdlinde merged 2 commits intoAug 31, 2026
Merged
Conversation
Four assignments of $config->temperature and one of $config->organizationId
on LLPhant\OpenAIConfig. Neither is a property of that class, so each one
created a DYNAMIC property that nothing ever read. An agent temperature
configured in the UI never reached the model.
The suppressions above them said so and were read the other way round:
@psalm-suppress UndefinedPropertyAssignment LLPhant dynamic properties
LLPhant does not use dynamic properties. It reads modelOptions:
OpenAIChat.php:89 $this->modelOptions = $config->modelOptions;
OpenAIChat.php:368 $openAiArgs = $this->modelOptions;
and OpenAIConfig own docblock lists temperature inside the ModelOptions
array shape, not among its properties. Nothing anywhere in the library
reads a temperature property. Temperature now goes to
$config->modelOptions["temperature"], which is what reaches the API.
organizationId is removed rather than relocated: LLPhant has no
organization support of any kind, so there is nowhere for it to go. That
drops the pretence, not the behaviour, because there was none.
⚠️ PHP 8.2 deprecated dynamic property creation, so this was also going to
start emitting deprecations and then fail outright.
Verified with the repo own vendor/bin/psalm 5.26.1 on PHP 8.3: errors with
the baseline emptied went 173 -> 168, regenerated baseline is green. PHPUnit
was NOT run here, it needs the Nextcloud server bootstrap that only exists
inside a server checkout.
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ❌ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-31 17:58 UTC
Download the full PDF report from the workflow artifacts.
phpcs: "Inline comments must start with a capital letter". Five of the comments I added opened on an identifier (modelOptions, organizationId).
Contributor
Quality Report — ConductionNL/openregister @
|
| Check | PHP | Vue | Security | License | Tests |
|---|---|---|---|---|---|
| lint | ✅ | ||||
| phpcs | ✅ | ||||
| phpmd | ✅ | ||||
| psalm | ✅ | ||||
| phpstan | ✅ | ||||
| phpmetrics | ✅ | ||||
| eslint | ✅ | ||||
| stylelint | ✅ | ||||
| build | ✅ | ||||
| check-specs | ✅ | ||||
| test-l10n | ✅ | ||||
| test-l10n-parity | ✅ | ||||
| format | ✅ | ||||
| check-schema-l10n | ✅ | ||||
| check-l10n-js | ✅ | ||||
| composer | ✅ | ✅ 174/174 | |||
| npm | ✅ | ✅ 543/543 | |||
| app:check-code | ⏭️ | ||||
| info.xml | ✅ | ||||
| REUSE | ❌ | ||||
| PHPUnit | ✅ | ||||
| Newman | ✅ | ||||
| Playwright | ⏭️ deferred — runs on the promotion into beta/main, not on a pull request into development | ||||
| Hydra gates | ✅ |
Quality workflow — 2026-08-31 18:08 UTC
Download the full PDF report from the workflow artifacts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four assignments of
$config->temperatureand one of$config->organizationIdonLLPhant\OpenAIConfig. Neither is a property of that class, so each created a dynamic property that nothing ever read. An agent temperature configured in the UI never reached the model.The suppressions sitting above them said as much, and had been read the wrong way round:
LLPhant does not use dynamic properties. It reads
modelOptions:and
OpenAIConfigs own docblock liststemperatureinside theModelOptionsarray shape, not among its properties. Nothing in the library reads atemperatureproperty anywhere. Temperature now goes to$config->modelOptions["temperature"], which is what actually reaches the API.organizationIdis removed rather than relocated: LLPhant has no organization support of any kind, so there is nowhere for it to go. That drops the pretence, not the behaviour, because there was none.Worth noting PHP 8.2 deprecated dynamic property creation, so this was heading for deprecation notices and then hard failure regardless.
Verification
vendor/bin/psalm5.26.1 on PHP 8.3: errors with the baseline emptied went 173 -> 168, regenerated baseline green. PHPUnit not run locally (needs the server bootstrap).