Skip to content

feat(integrations): smart config merging for Codex and improve Chines…#249

Merged
jundot merged 5 commits intojundot:mainfrom
JasonYeYuhe:feat/codex-integration-and-zh-docs
Mar 21, 2026
Merged

feat(integrations): smart config merging for Codex and improve Chines…#249
jundot merged 5 commits intojundot:mainfrom
JasonYeYuhe:feat/codex-integration-and-zh-docs

Conversation

@JasonYeYuhe
Copy link
Copy Markdown
Contributor

@JasonYeYuhe JasonYeYuhe commented Mar 16, 2026 •
edited
Loading

PR Description: feat(integrations): smart config merging for Codex and improve Chinese README

Overview

This PR introduces two main improvements to enhance the developer experience, especially for users utilizing advanced reasoning models and regional mirrors.

1. Refactor Codex Integration (omlx/integrations/codex.py)

  • Smart Configuration Merging: Replaced the previous "overwrite everything" logic with a non-destructive merging strategy for ~/.codex/config.toml. It now preserves existing user settings (like trust_level or other model providers) and only updates or inserts the [model_providers.omlx] section.
  • Reasoning Model Support: Added automatic detection for reasoning-capable models. If the model name contains keywords like thinking, o1, o3, or r1, it automatically configures model_reasoning_effort = "high" in the generated config. This is a crucial feature for the latest models (like GPT-5.4 or Qwen reasoning variants).

2. Documentation Improvements (README.zh.md)

  • Missing Features: Added the --hf-endpoint CLI example which was present in the English version but missing in the Chinese translation. This is vital for users in regions with restricted access to HuggingFace.
  • Translation Alignment: Synchronized the "Acknowledgments" and "Integrations" sections with the latest English version to ensure 100% consistency across languages.
  • Fixed Anchor Links: Corrected several internal links and updated the OS requirement to macOS 15.0+ (Sequoia) to match the English README.

Testing Conducted

  • Config Merge Test: Verified that existing settings in ~/.codex/config.toml are correctly preserved when oMLX is configured.
  • Reasoning Flag Test: Confirmed that model_reasoning_effort = "high" is correctly injected for models with thinking or o1/o3/r1 in their names.
  • Documentation: Manually reviewed the rendered Markdown of README.zh.md to ensure correct formatting and accuracy.

jundot
jundot reviewed Mar 16, 2026
View reviewed changes
Copy link
Copy Markdown
Owner

@jundot jundot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the codex config merging work and the README sync. Few things:

1. PR_DESCRIPTION.md should not be committed to the repo. Please remove it.

2. TOML merge bug: the code uses not in_omlx_section to identify top-level keys, but this also matches keys inside other provider sections. A model key under [model_providers.custom] would get overwritten. You need to track whether you're inside any section, not just the omlx one.

3. Reasoning model detection uses raw substring matching ("o1", "r1", etc), so names like "modelo1x" would false-positive. A word boundary pattern would be safer.

4. Missing tests for the new merge logic. OpenCode and OpenClaw both have test_configure_preserves_existing style tests. The new codex merge should have similar coverage.

@JasonYeYuhe
Copy link
Copy Markdown
Contributor Author

JasonYeYuhe commented Mar 17, 2026

Thanks for the thorough review, jundot! I've pushed an update that addresses all your points:

  1. Removed PR_DESCRIPTION.md from the repo.
  2. Fixed the TOML merge logic to correctly track in_any_section, ensuring keys in non-omlx sections remain untouched.
  3. Updated the reasoning model detection to use regex word boundaries to prevent false positives.
  4. Added test_configure_preserves_existing to TestCodexIntegration for test coverage, mirroring the style used in OpenCode and OpenClaw.

Let me know if anything else is needed!

@jundot jundot force-pushed the main branch 7 times, most recently from f6faf2f to c2beead Compare March 21, 2026 05:58
@jundot
Copy link
Copy Markdown
Owner

jundot commented Mar 21, 2026

Thanks for the clean update, everything from the first review is addressed properly.

I'll merge this now and do a small follow-up commit for a couple of minor things i noticed (dead template cleanup, reasoning flag edge case). Nothing major, just housekeeping on top of your work.

Appreciate the contribution!

@jundot jundot merged commit 56fd63c into jundot:main Mar 21, 2026
jundot added a commit that referenced this pull request Mar 21, 2026
@jundot
fix: clean up codex config merge from #249
2455c2c 
- remove unused _CONFIG_TEMPLATE
- move `import re` to module level
- clear stale model_reasoning_effort when switching to non-reasoning model
- add tests for reasoning model detection and flag cleanup
@JasonYeYuhe
Copy link
Copy Markdown
Contributor Author

JasonYeYuhe commented Mar 28, 2026

Thank you for the merge. Appreciate your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jundot jundot jundot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JasonYeYuhe @jundot