[CI only] validate #3359 (do not merge) - #3613
Closed
nicoloboschi wants to merge 2 commits into
Closed
Conversation
HindsightEmbedded previously defaulted llm_api_key to an empty string and always forwarded it to the daemon. Omitting the argument therefore replaced a valid key inherited from the parent environment or profile. Use None as the default and omit the config entry only in that case. Keep an explicit empty string as an override, so local LLM services can still run without authentication. Add regression coverage for omitted, empty, and non-empty keys.
Extends the llm_api_key fix to every LLM/daemon setting HindsightEmbedded
forwards. llm_provider, llm_model, log_level and idle_timeout were still sent
unconditionally, so a client constructed without them overwrote whatever the
profile had configured -- and _register_profile then persisted the placeholders
into the profile's .env file. A profile set up for anthropic came back as
groq / openai-gpt-oss-120b on disk after a single keyless HindsightEmbedded
call, which left the newly inherited key pointed at the wrong provider.
All five settings now default to None and are omitted from the daemon config
when not passed, so the daemon resolves them from the profile .env, then the
parent environment, then its own defaults. Those defaults already match the
values that were being hardcoded for log_level ("info") and idle_timeout (0).
Dropping the hardcoded "groq" / "openai/gpt-oss-120b" means a client that
configures nothing anywhere now lands on the server's default provider
(openai); every documented example passes llm_provider explicitly.
Moves the config coverage into test_embedded_config.py, which drives the real
embed-manager start path with Popen stubbed and asserts on the environment the
daemon child actually receives, not just on the intermediate config dict.
Collaborator
Author
|
CI validated the branch of #3359, which is merged. Closing. |
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
2 participants
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.
Throwaway PR so the secret-gated CI jobs run against the branch of #3359, which is a fork PR and therefore skips them.
Merge #3359, not this. This branch will be deleted once CI reports.