fix(llm): refresh demo provider configs on page load by JesusMan0529 · Pull Request #378 · apache/hugegraph-ai

JesusMan0529 · 2026-10-05T10:42:26Z

Fixes #91.

When .env changes while the demo is running, refreshing the page reloads the shared LLM settings and updates all five provider forms, including when the provider type stays the same. The SiliconFlow form honors the configured model. Invalid or failed reloads retain the previous settings, show a warning, and still render the forms and Apply buttons.

Settings load directly from the current file without copying values into os.environ, so deleted keys use genuine process settings or defaults. Standalone file settings remain supported through an explicit dotenv lookup: the Thin API write opt-in, development reload, and NLTK data/cache paths. The Thin API still requires login and bearer authentication for writes.

Provider functions and factories capture the provider type and parameters together under runtime_config_lock, including chat, extraction, Text2Gremlin, embedding, and reranking clients. Client construction and embedding dimension probes run after releasing the lock. Provider form rendering uses the same lock for its shared settings updates. The two LiteLLM embedding entry points now supply the required initial dimension.

Validation:

Regression tests use the real settings loader and Gradio page-load callback. They pause a reload midway through its field assignments and exercise all nine provider entry points with LiteLLM and Ollama configurations; additional tests cover delayed factory use and a refresh during embedding dimension detection.
File-only Thin API tests exercise both authenticated production routes without exporting the write flag, including source precedence and removal. Other standalone dotenv consumers and the real LiteLLM constructors are covered.
The 37 failing follow-up cases were reproduced before their fixes (32 for the two new review findings and related factory/flag behavior, then 5 for the additional source-audit findings). The existing 13 page-refresh cases remain covered.
The complete local LLM unit/contract selection reported 486 passed, 3 skipped, 18 deselected, with 57.13% coverage against the 34% requirement. Windows then exited with 0xC0000005, as previously reproduced on the original PR head; this local run is not a clean process exit.
Root Ruff format/lint and git diff --check pass.
The non-blocking local ty check retains 726 diagnostics, down from 728 before this follow-up. Comparing file/rule/message pairs found no new diagnostics; the two missing embedding-dimension arguments were removed.
Live provider calls, real-service integration, and browser end-to-end tests were not run locally. Upstream CI results are reported separately for the submitted commit.

Upstream validation for 76a8ea04dbc41969dcfb7e49e6ccb4243b6da7bb: all 20 checks succeeded, including the LLM unit/contract, HugeGraph boundary and core smoke jobs on Python 3.10/3.11, client checks, builds, Ruff and security/license checks. The Linux Python 3.11 unit/contract log confirms 486 passed, 3 skipped and 18 deselected with a normal exit. The ty jobs remain configured as non-blocking. CI run.

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

JesusMan0529 · 2026-10-05T10:43:04Z

😊

bitflicker64

Blocking: yes. Summary: The page-load reload works for valid .env edits, but the five provider renders lost their implicit load trigger, so any .env value that LLMConfig rejects leaves the Chat, mini_tasks, text2gql, Embedding and Reranker sections empty after a refresh. Score 7/10. Evidence: read the full exact-head diff, BaseConfig.__init__, LLMConfig field types and gradio 5.20.1 renderable.py; ran uv run pytest hugegraph-llm/src/tests/config/test_demo_config_refresh.py (3 passed); replayed the load event with CHAT_LLM_TYPE=ollama in .env and saw refresh_llm_config raise a pydantic literal_error, with only vector_engine_settings rendering. All latest-head checks pass.

bitflicker64 · 2026-10-05T15:01:33Z


+    def refresh_llm_config(revision):
+        with runtime_config_lock:
+            llm_settings.__init__()  # type: ignore[misc] # pylint: disable=C2801


Important: If .env holds any value LLMConfig rejects, the page loads with every provider form missing. With explicit triggers, gradio 5.20 no longer adds the implicit page-load trigger to @gr.render (gradio/renderable.py, lines 137-150). The five forms now render only when config_revision changes, and that happens only when this function returns. When llm_settings.__init__() raises, the load event fails, config_revision stays at 0, and Chat, mini_tasks, text2gql, Embedding and Reranker show no fields and no Apply button. Before this PR, those renders also ran on load and showed the in-memory values.

Reproduced on this head with gradio 5.20.1: I wrote a .env that differs only by CHAT_LLM_TYPE=ollama (the valid value is ollama/local) and replayed the load event the way test_demo_config_refresh.py does. refresh_llm_config raised a pydantic literal_error, and the only render that fired was vector_engine_settings. A mistyped Literal value or a non-integer token or port count is a likely mistake in the edit-then-refresh workflow this PR adds, and it now removes the config UI instead of leaving the old values editable.

Please keep the renders firing when the reload fails and tell the user why, for example:

def refresh_llm_config(revision): with runtime_config_lock: try: llm_settings.__init__() # type: ignore[misc] # pylint: disable=C2801 except Exception as e: # pylint: disable=broad-except log.error("Failed to reload LLM settings from %s: %s", env_path, e) gr.Warning(f"Could not reload {env_path}, keeping the current settings: {e}") return (

Also add a case to test_demo_config_refresh.py with an invalid value, and assert that the provider textboxes still render.

Thanks for catching this! Fixed in e5c4cba. A failed reload now keeps the current settings, shows a warning, and still renders all five provider forms and their Apply buttons. Added regression tests for invalid provider/token/port values and recovery after correcting the file; all 13 refresh tests pass.

imbajin

Blocking: no. Summary: Refreshing after removing an LLM setting can restore the stale value from the previous .env contents. Evidence: On this head the page-load callback reinitializes llm_settings, while BaseConfig copies .env entries into os.environ and writes missing model fields back to the file.

imbajin · 2026-10-05T17:19:49Z


+    def refresh_llm_config(revision):
+        with runtime_config_lock:
+            llm_settings.__init__()  # type: ignore[misc] # pylint: disable=C2801


⚠️ Blocking: no. Summary: Refreshing after removing a previously configured key can restore its stale value and write it back to .env. Please reload without retaining values copied from earlier .env contents, and cover key removal in the regression test. Evidence: BaseConfig.init copies .env entries into os.environ without clearing entries absent on later loads; check_env() then writes missing model fields back to the file.

i got it 😊

Fixed in e5c4cba. Reloads now read .env directly without copying its values into os.environ, so deleted keys fall back to process settings or defaults. Added regression tests for removing model, API key and provider settings, including the environment fallback.

imbajin

Blocking: no. Summary: The exact-head review found two runtime regressions in configuration reload. Evidence: the new dotenv source no longer exposes the standalone Thin API write flag to os.getenv, and provider construction can read the shared settings object while it is only partially refreshed.

imbajin · 2026-10-06T09:17:06Z

-                    os.environ[k] = v
-
-            # Step 2: Init the parent class with loaded environment variables
+            # Load and validate settings directly from the current sources.


⚠️ Blocking: no. Summary: Removing the .env-to-os.environ copy also drops HUGEGRAPH_LLM_ENABLE_THIN_WRITES: thin_api._thin_write_disabled() still reads it with os.getenv, but this key is not a BaseConfig field and is ignored by the settings source. A deployment that stores this documented flag only in .env will leave POST /graph-import and /vid-embeddings/refresh disabled. Please load this standalone flag through a settings field or dotenv lookup, and add a regression test without exporting it into the process environment. Evidence: BaseConfig.init no longer publishes arbitrary .env keys; thin_api.py:115 reads this flag directly from the process environment; hugegraph-llm/README.md:198 documents the opt-in.

Fixed in 76a8ea0. The .env-only write opt-in now works for both endpoints without exporting it into the process environment. Added production-router tests for authentication, source precedence and removing the flag. Also checked the other standalone file settings and preserved their dotenv support.

imbajin · 2026-10-06T09:17:06Z

+        with runtime_config_lock:
+            try:
+                reloaded = type(llm_settings)()
+                _restore_config(llm_settings, reloaded.model_dump())


⚠️ Blocking: no. Summary: This reload assigns each field on the shared llm_settings object under runtime_config_lock, but provider construction in init_llm.py reads that object without taking the lock. A request overlapping a page-load refresh can observe the new provider type with old model, endpoint, or credential values and fail to construct its client. Please make provider creation use a consistent settings snapshot synchronized with reloads, and add a focused concurrent read/reload regression check. Evidence: _restore_config sets fields sequentially at this line; get_chat_llm and the other provider constructors read type and configuration fields separately in hugegraph_llm/models/llms/init_llm.py.

fix(llm): refresh demo provider configs on page load

b629fbb

Copilot AI balanced review requested due to automatic review settings October 5, 2026 10:42

Copilot AI reviewed Oct 5, 2026

github-actions Bot added the llm label Oct 5, 2026

bitflicker64 reviewed Oct 5, 2026

View reviewed changes

imbajin reviewed Oct 5, 2026

View reviewed changes

imbajin mentioned this pull request Oct 6, 2026

[1.8.0] Release TODOs and cross-repository integration checklist apache/hugegraph#3242

Open

18 tasks

fix(llm): preserve config forms on failed reloads

e5c4cba

imbajin reviewed Oct 6, 2026

View reviewed changes

fix(llm): preserve dotenv flags and synchronize provider reads

76a8ea0

JesusMan0529 closed this Oct 6, 2026

Conversation

JesusMan0529 commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

JesusMan0529 commented Oct 5, 2026

Uh oh!

bitflicker64 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imbajin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JesusMan0529 Oct 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

imbajin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JesusMan0529 commented Oct 5, 2026 •

edited

Loading

JesusMan0529 Oct 6, 2026 •

edited

Loading