Another round of improvements for config error messages#9746
Merged
etraut-openai merged 3 commits intomainfrom Jan 24, 2026
Merged
Another round of improvements for config error messages#9746etraut-openai merged 3 commits intomainfrom
etraut-openai merged 3 commits intomainfrom
Conversation
I recently made some improvements to config error messages, but there's still room for improvement. For example, see #9668 where the user was understandably confused about the source of the problem and how to fix it. The improved error message: 1. Clearly identifies the config file where the error was found (which is more important now that we support layered configs) 2. Provides a line and column number of the error 3. Displays the line where the error occurred and underlines it For example, here's the current CLI error message: ``` Error loading config.toml: unknown variant `xxhigh`, expected one of `none`, `minimal`, `low`, `medium`, `high`, `xhigh` in `model_reasoning_effort` ``` And here's the improved message: ``` Error loading config.toml: /Users/etraut/.codex/config.toml:2:26: unknown variant `xxhigh`, expected one of `none`, `minimal`, `low`, `medium`, `high`, `xhigh` | 2 | model_reasoning_effort = "xxhigh" | ^^^^^^^^ ``` In addition, this PR adds the file name and text range to the ConfigWarningNotification app server struct. This will allow GUI clients to present the user with a better error message. (This was a suggestion from @.bolinfest when he reviewed my previous PR.)
Collaborator
Author
|
@codex review |
Contributor
|
Codex Review: Didn't find any major issues. Bravo. ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
bolinfest
approved these changes
Jan 24, 2026
Collaborator
bolinfest
left a comment
bolinfest
left a comment
There was a problem hiding this comment.
Very nice and thoughtful change!
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
In a recent PR, I made some improvements to config error messages so errors didn't leave app server clients in a dead state. This is a follow-on PR to make these error messages more readable and actionable for both TUI and GUI users. For example, see #9668 where the user was understandably confused about the source of the problem and how to fix it.
The improved error message:
For example, if my
config.tomlincludes the following:Here's the current CLI error message:
And here's the improved message:
The bulk of the new logic is contained within a new module
config_loader/diagnostics.rsthat is responsible for calculating the text range for a given toml path (which is more involved than I would have expected).In addition, this PR adds the file name and text range to the
ConfigWarningNotificationapp server struct. This allows GUI clients to present the user with a better error message and an optional link to open the errant config file. This was a suggestion from @.bolinfest when he reviewed my previous PR.