Skip to content

fix(chat): send temperature through modelOptions, where LLPhant reads it - #3243

Merged
rubenvdlinde merged 2 commits into
developmentfrom
fix/llm-temperature-was-silently-discarded
Aug 31, 2026
Merged

fix(chat): send temperature through modelOptions, where LLPhant reads it#3243
rubenvdlinde merged 2 commits into
developmentfrom
fix/llm-temperature-was-silently-discarded

Conversation

@rubenvdlinde

Copy link
Copy Markdown
Contributor

Four assignments of $config->temperature and one of $config->organizationId on LLPhant\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:

@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 OpenAIConfigs own docblock lists temperature inside the ModelOptions array shape, not among its properties. Nothing in the library reads a temperature property anywhere. Temperature now goes to $config->modelOptions["temperature"], which is what actually 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.

Worth noting PHP 8.2 deprecated dynamic property creation, so this was heading for deprecation notices and then hard failure regardless.

Verification

vendor/bin/psalm 5.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).

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.
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ 9552f21

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).
@github-actions

Copy link
Copy Markdown
Contributor

Quality Report — ConductionNL/openregister @ c180221

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.

@rubenvdlinde
rubenvdlinde merged commit 290b35d into development Aug 31, 2026
47 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant