[YellowSnnowmann]

Summary

• Added guard logic in config loading to apply env overrides only when values are valid and intentional.
• Prevented empty/invalid env values from clobbering file-based config defaults.
• Hardened src/openhuman/config/schema/load.rs behavior around override precedence and fallback safety.
• Expanded src/openhuman/config/schema/load_tests.rs with regression coverage for override edge cases.
• Reduced risk of silent misconfiguration caused by partial or malformed environment setup.

Problem

• Config loading previously allowed risky env override paths where invalid or blank values could override known-good config.
• This made runtime behavior depend on fragile environment state and could cause hard-to-debug startup/config issues.
• Reviewers needed a clear, test-backed contract for when env should win vs when loader should keep base config values.

Solution

• Added explicit validation/guard checks in the load pipeline before applying env-driven overrides.
• Kept precedence model intact (env over file) only when env values pass guard conditions.
• Preserved existing defaults and file values when override inputs are missing/blank/invalid.
• Added focused tests for:
  • valid override application
  • invalid/empty override rejection
  • fallback to base config behavior
• Tradeoff: stricter guards may ignore previously accepted-but-bad env inputs, but this is intentional to improve safety and predictability.

Submission Checklist

• If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.
• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
• Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/coverage.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
• Coverage matrix updated — added/removed/renamed feature rows in docs/TEST-COVERAGE-MATRIX.md reflect this change (or N/A: behaviour-only change)
• All affected feature IDs from the matrix are listed in the PR description under ## Related
• No new external network dependencies introduced (mock backend used per Testing Strategy)
• Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md)
• Linked issue closed via Closes #NNN in the ## Related section

Impact

• Runtime/platform impact: Rust core config initialization path (desktop core startup/runtime config behavior).
• Compatibility impact: safer env precedence semantics; invalid override inputs no longer silently override base config.
• Performance impact: negligible; only lightweight guard checks during config load.
• Security/reliability impact: lower misconfiguration risk and more deterministic startup behavior.

Related

• Closes: OPENHUMAN-TAURI-J8

Summary by CodeRabbit

• Bug Fixes
  • Environment variable handling tightened: the app now exclusively honors OPENHUMAN_MODEL for overriding the default model (ignores bare MODEL), and ignores whitespace-only OPENHUMAN_MODEL values to avoid accidentally clearing the configured default.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f10ab925-489d-407c-bc37-d3375d7227cf

📥 Commits

Reviewing files that changed from the base of the PR and between 77fa545 and e795926.

📒 Files selected for processing (2)

• src/openhuman/config/schema/load.rs
• src/openhuman/config/schema/load_tests.rs

---

📝 Walkthrough

Walkthrough

The PR removes the unprefixed MODEL alias from config env overlay, so only OPENHUMAN_MODEL (trimmed, non-empty) can override default_model; tests updated to assert this behavior.

Changes

Environment Variable Precedence for Model Configuration

Layer / File(s)	Summary
Namespaced OPENHUMAN_MODEL precedence and test src/openhuman/config/schema/load.rs, src/openhuman/config/schema/load_tests.rs	Config::apply_env_overlay_with removes the fallback to unprefixed MODEL and now reads only OPENHUMAN_MODEL (trimmed, ignored if empty). Tests updated to verify OPENHUMAN_MODEL wins when both variables are set, bare MODEL does not override default_model, and whitespace-only OPENHUMAN_MODEL is ignored.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Poem

I’m a rabbit in the config glade,
I trim the leaves that envs have made,
Only OPENHUMAN_MODEL gets my nod,
Bare MODEL is politely shod,
Hoppity-hop—defaults kept unawed 🐰

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'fix(config): guard env overrides during config load' accurately and specifically describes the main change: adding guard logic to environment variable overrides during config loading.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/openhuman/config/schema/load_tests.rs (1)

426-447: ⚡ Quick win

Add a whitespace-only OPENHUMAN_MODEL regression assertion here.

This test already covers precedence/alias behavior; adding " " keeps guard behavior locked so blank-like values cannot override default_model.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/openhuman/config/schema/load_tests.rs` around lines 426 - 447, In the
test env_overlay_model_only_honours_namespaced_var, add a case that ensures a
whitespace-only OPENHUMAN_MODEL value does not override Config::default_model:
create a HashMapEnv with ("OPENHUMAN_MODEL", "   "), call
cfg.apply_env_overlay_with(&env) on a fresh Config (or compare to a saved
original = cfg.default_model.clone()), and assert cfg.default_model equals the
original; this uses the same function names
(env_overlay_model_only_honours_namespaced_var, Config::apply_env_overlay_with,
default_model, HashMapEnv) so the blank-like OPENHUMAN_MODEL cannot clobber the
model.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/openhuman/config/schema/load.rs`:
- Around line 1000-1003: The current branch sets self.default_model from
env.get("OPENHUMAN_MODEL") without trimming, allowing whitespace-only values;
update the logic around env.get("OPENHUMAN_MODEL")/model so you .trim() the
retrieved string and only assign Some(trimmed) to self.default_model when the
trimmed string is non-empty (i.e., check trimmed.is_empty() before setting);
keep the same variable names (model, self.default_model) and the same
conditional structure but use the trimmed value for the emptiness check and
assignment.

---

Nitpick comments:
In `@src/openhuman/config/schema/load_tests.rs`:
- Around line 426-447: In the test
env_overlay_model_only_honours_namespaced_var, add a case that ensures a
whitespace-only OPENHUMAN_MODEL value does not override Config::default_model:
create a HashMapEnv with ("OPENHUMAN_MODEL", "   "), call
cfg.apply_env_overlay_with(&env) on a fresh Config (or compare to a saved
original = cfg.default_model.clone()), and assert cfg.default_model equals the
original; this uses the same function names
(env_overlay_model_only_honours_namespaced_var, Config::apply_env_overlay_with,
default_model, HashMapEnv) so the blank-like OPENHUMAN_MODEL cannot clobber the
model.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 949e53b0-dde1-4673-83dd-075017387f76

📥 Commits

Reviewing files that changed from the base of the PR and between 4384cd1 and 77fa545.

📒 Files selected for processing (2)

• src/openhuman/config/schema/load.rs
• src/openhuman/config/schema/load_tests.rs