Skip to content

refactor: replace process.env with getEnvOptions() in graphile-llm#1011

Merged
pyramation merged 2 commits intomainfrom
devin/1776586895-graphile-llm-getenv
Apr 19, 2026
Merged

refactor: replace process.env with getEnvOptions() in graphile-llm#1011
pyramation merged 2 commits intomainfrom
devin/1776586895-graphile-llm-getenv

Conversation

@pyramation
Copy link
Copy Markdown
Contributor

Summary

Migrates graphile-llm's buildEmbedderFromEnv() and buildChatCompleterFromEnv() from direct process.env reads to the repo-standard getEnvOptions() system from @constructive-io/graphql-env. This ensures LLM config follows the same merge hierarchy (pgpm defaults → config file → env vars → overrides) used by all other Constructive packages.

Changes across 3 packages:

  • @constructive-io/graphql-types: New LlmOptions / LlmEmbedderOptions / LlmChatOptions interfaces; added llm?: LlmOptions to ConstructiveOptions
  • @constructive-io/graphql-env: Parses EMBEDDER_PROVIDER, EMBEDDER_MODEL, EMBEDDER_BASE_URL, CHAT_PROVIDER, CHAT_MODEL, CHAT_BASE_URL into the structured llm config object
  • graphile-llm: embedder.ts and chat.ts now call getEnvOptions() instead of reading process.env directly; added @constructive-io/graphql-env as a dependency

Review & Testing Checklist for Human

  • Verify existing env-based tests still pass: buildEmbedderFromEnv / buildChatCompleterFromEnv tests still set process.env directly. This works because getEnvOptions() reads from process.env under the hood, but confirm no config file in the test environment interferes (since getEnvOptions now also calls loadConfigSync(cwd))
  • Check the env.ts guard logic: The outer guard (EMBEDDER_PROVIDER || CHAT_PROVIDER) means setting only EMBEDDER_MODEL or CHAT_BASE_URL without a provider will silently produce no llm config. Confirm this is the desired behavior vs. raising a warning
  • Confirm no snapshot test breakage in graphql/env/__tests__/ from the new llm field on the return type

Notes

  • The pnpm-lock.yaml diff is mostly YAML formatting noise (single-line vs multi-line resolution fields) — the only semantic change is the new @constructive-io/graphql-env workspace link for graphile-llm
  • No new tests were added for the getGraphQLEnvVars LLM parsing itself — existing tests exercise it indirectly through the graphile-llm test suite

Link to Devin session: https://app.devin.ai/sessions/720693f565cc4b8fb4b198ddf75814cc
Requested by: @pyramation

- Add LlmOptions type to @constructive-io/graphql-types (llm.ts)
- Add EMBEDDER_*/CHAT_* env var parsing to @constructive-io/graphql-env
- Update embedder.ts and chat.ts to use getEnvOptions() instead of process.env
- Add @constructive-io/graphql-env as dependency of graphile-llm
@devin-ai-integration
Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@pyramation pyramation merged commit 769d49c into main Apr 19, 2026
51 checks passed
@pyramation pyramation deleted the devin/1776586895-graphile-llm-getenv branch April 19, 2026 08:35
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