Skip to content

fix(config): report an unknown lakeview model as a ConfigError - #486

Open
sxh313 wants to merge 1 commit into
bytedance:mainfrom
sxh313:fix/lakeview-model-not-found
Open

sxh313 wants to merge 1 commit into
bytedance:mainfrom
sxh313:fix/lakeview-model-not-found

Conversation

@sxh313

@sxh313 sxh313 commented Sep 25, 2026

Copy link
Copy Markdown

Description

Config.create() looks the lakeview model up with a bare dict index, so a lakeview.model name that is not defined under models: aborts config loading with KeyError: '<name>' instead of the ConfigError this function raises for every other bad model reference.

lakeview_model = config_models[lakeview_model_name]   # trae_agent/utils/config.py:262

The same block two lines above already promises a ConfigError:

lakeview_model_name = lakeview.get("model", None)
if lakeview_model_name is None:
    raise ConfigError("No model provided for lakeview")
lakeview_model = config_models[lakeview_model_name]   # <- leaks KeyError if the name is unknown

and the sibling lookup in the very same function wraps the identical failure:

try:
    agent_model = config_models[agent_model_name]
except KeyError as e:
    raise ConfigError(f"Model {agent_model_name} not found") from e

This makes the fix the same two lines, with the same message, so the only unwrapped model lookup in Config.create() now behaves like the other two. Measured on main (e839e55), changing only which section carries the typo:

section with an unknown model name result on main result on this branch
agents.trae_agent.model ConfigError: Model X not found ConfigError: Model X not found (unchanged)
lakeview.model KeyError: 'X' ConfigError: Model X not found

trae-cli run, trae-cli interactive and trae-cli show-config all call Config.create(...) with no try/except (trae_agent/cli.py:294, :459, :650), so the bare KeyError traceback is what a user gets - and it does not mention lakeview, models, or the config file.

More Information

Resolves #485.

The regression tests go into a new file, tests/utils/test_config_model_lookup.py, rather than into tests/utils/test_config.py: that file already carries the TestLakeviewConfig and TestConfigBaseURL cases and is being edited by five other open PRs (four of them at its import block), so a new focused file keeps this diff free of incidental conflicts. Two of the three tests are green on main on purpose - they pin the happy path (lakeview.model that exists still loads) and the neighbouring ConfigError("No model provided for lakeview") branch, so neither can be broken by this change.

Nothing here changes behaviour for a valid config: the wrapped lookup returns the same object, and raise ... from e keeps the original KeyError in the chain for anyone debugging.

This is not a regression. git log --all -S'lakeview_model = config_models' returns only 6fc646d New Configuration System backed by YAML (#234) and c924aab refactor(config): move legacy config conversion to Config class, so the lookup has been unwrapped since the YAML config system landed.

Two things this deliberately does not touch, so the diff stays about one defect:

  • models.<name> entries that omit model_provider still raise KeyError: 'model_provider' from config.py:246 (model_config["model_provider"]). Same family, different line; it needs its own report.
  • trae_agent/tools/base.py:229-236 renders tool exceptions as f"...: {str(e)}", which drops the exception type. Worth knowing because it is how this error reaches an agent mid-run, but changing it would affect every tool.

Because a .github/workflows/* rule limits the repo's workflows to the upstream repository, CI on a fork PR stops at action_required and never runs; everything below was executed locally on Windows.

Validation

All commands run against main (e839e559ac61bdd0e057c375dd1dee391fee797d).

1. Red - the new tests on unmodified source. Worktree at e839e55 containing only the new test file, interpreter from the project venv with PYTHONPATH pointed at the worktree (import trae_agent self-checked as USING D:\Desktop\pr\trae-wt-lakeview-2143\trae_agent):

$ SKIP_OLLAMA_TEST=true SKIP_OPENROUTER_TEST=true SKIP_GOOGLE_TEST=true python -m pytest tests/utils/test_config_model_lookup.py --tb=line -v
collected 3 items
tests\utils\test_config_model_lookup.py ..F                              [100%]
D:\Desktop\pr\trae-wt-lakeview-2143\trae_agent\utils\config.py:262: KeyError: 'missing_model'
========================= 1 failed, 2 passed in 1.62s =========================

2. Green - after the change.

$ SKIP_OLLAMA_TEST=true SKIP_OPENROUTER_TEST=true SKIP_GOOGLE_TEST=true python -m pytest tests/utils/test_config_model_lookup.py --tb=short -v
tests\utils\test_config_model_lookup.py ...                              [100%]
============================== 3 passed in 1.42s ==============================

3. End-to-end, no mocks - the same config file read by the same command on both trees. lakeview-demo.yaml declares enable_lakeview: true and a lakeview.model that is not in models::

# main (clean worktree at e839e55)
KeyError: 'lakeview_model_xxxx'
caught as ConfigError? False

# this branch
ConfigError: Model lakeview_model_xxxx not found
caught as ConfigError? True

Full traceback on main:

Traceback (most recent call last):
  File "...\trae_agent\utils\config.py", line 262, in create
    lakeview_model = config_models[lakeview_model_name]
KeyError: 'lakeview_model_xxxx'

4. Full suite, branch vs. a clean-main control run in this same session. Control is a separate worktree checked out at e839e55 (not the shared clone), so the comparison is apples to apples:

branch:   3 failed, 62 passed, 17 skipped   (59 baseline + the 3 tests added here)
control:  3 failed, 59 passed, 17 skipped

The three failures are the identical Windows-only tests/tools/test_bash_tool.py cases on both trees (diff of the two FAILED lists is empty):

FAILED tests/tools/test_bash_tool.py::TestBashTool::test_command_error_handling
FAILED tests/tools/test_bash_tool.py::TestBashTool::test_session_restart
FAILED tests/tools/test_bash_tool.py::TestBashTool::test_successful_command_execution

They are a pre-existing main baseline on Windows (pexpect / bash), unrelated to this change, and they do not fail on the Ubuntu CI runner.

5. Lint and type checks - pre-commit run --files trae_agent/utils/config.py tests/utils/test_config_model_lookup.py:

trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check for added large files..............................................Passed
detect private key.......................................................Passed
ruff (legacy alias)......................................................Passed
ruff format..............................................................Passed
codespell................................................................Passed
mypy.....................................................................Failed
trae_agent\tools\bash_tool.py:49: error: Module has no attribute "setsid"  [attr-defined]
Found 1 error in 1 file (checked 2 source files)

The single mypy error is in trae_agent/tools/bash_tool.py, which this PR does not touch: os.setsid is POSIX-only and does not exist on Windows, so this line errors on every local mypy run on this platform (checked 2 source files - neither of the two files in this diff produced an error). It will not appear on the Ubuntu CI runner.

6. Conflict check against the open PRs that also edit config.py. trae_agent/utils/config.py is touched by #434/#435/#436/#437, #412 and #409, so each was fetched and test-merged against this branch:

git merge-tree --write-tree fix/lakeview-model-not-found <pr-head>
  #434 -> 0    #437 -> 0    #412 -> 0
  #435 -> 0    #436 -> 0    #409 -> 0

Exit code 0 means a clean merge. Their hunks in create() start at line 269 (allow_mcp_servers); this diff ends at 262.

Linked Issues

Resolves #485

@sxh313

sxh313 commented Sep 25, 2026

Copy link
Copy Markdown
Author

Closing this PR. The reason is the state of the repository, not the change.

Measured against bytedance/trae-agent at 2026-09-25T23:26Z, re-read fresh before acting:

  1. Pull requests merged in the last 30 days: 0. GET /search/issues?q=repo:bytedance/trae-agent is:pr is:merged merged:>=2026-08-27 → total_count = 0. The newest merged PR anywhere in the repository is fix(openai): persist tool outputs in message history #369, merged_at = 2026-02-05T11:21:00Z.
  2. main has exactly one commit in the last twelve months. GET /repos/bytedance/trae-agent/commits?sha=main&since=2025-09-26 → one commit, e839e559a, 2026-02-05 (the merge of fix(openai): persist tool outputs in message history #369). This PR's own base.sha is that same e839e559a, i.e. the tip it was written against is still the tip.
  3. Nothing about this change has ever been machine-validated here. GET /repos/bytedance/trae-agent/commits/eb9b5047bdfaccc41273a37b7f6e7dab31018624/check-runs → total_count = 0. The two runs created for this head, Pre-commit 36195867928 and Unit Tests 36195868072 (created 2026-09-25T22:16:12Z), are completed / action_required with GET /actions/runs/<id>/jobs → total_count = 0 for each — no job, no step, no log. A maintainer has to approve a fork PR's first run before anything executes, and that approval is exactly what a repository with zero merges since February does not supply.

About the red ✗ that will appear on this PR within seconds of the close: on the two earlier batches in this repo today (five PRs closed at 18:22Z, five at 22:32Z), closing moved each head's two parked runs from action_required to completed/failure one to two seconds after closed_at, while jobs total_count stayed 0. It is the close being reported against runs that never started, not a test result. I am stating it here so nobody has to re-derive it.

Why this is a close and not a withdrawal — and why there are now eleven of these. This PR was opened at 22:16:08Z, four hours after #464-#472 were closed and during the same evening as #476/#478/#480/#482/#484. Ten of mine had already been closed on this repository today on these same three measurements (five at 18:22Z, five at 22:32Z); this one makes eleven, and #488 (23:15:00Z) follows. The changes are not the problem; the repository has no path from "PR opened" to "PR merged" for anyone right now, and leaving unreviewable PRs open hides that from everyone, including the people who might revive the project.

The branch sxh313/trae-agent:fix/lakeview-model-not-found stays and gh pr reopen restores this thread. main has not moved since this PR's base, so nothing upstream has addressed the case it covers — an unknown lakeview model name not surfacing as a ConfigError. If the project becomes active again, rebase onto a live main and this is worth a second look rather than a rewrite.

@sxh313

sxh313 commented Sep 26, 2026

Copy link
Copy Markdown
Author

Reopening at the lane owner's instruction, 2026-09-26.

Re-checked against bytedance/trae-agent at main = e839e559a (the commit this PR is based on; main has not moved since): the defect in #485 is still present and nothing upstream addresses it. The branch fix/lakeview-model-not-found is unchanged, still applies cleanly, and carries the regression test described above.

My closing comment recorded why the thread was parked - this repository has accepted no pull requests since 2026-02-05, so fork CI runs here stay at action_required and nothing about this change can be machine-validated - not that the change was wrong or the report was unfounded. Leaving the fix linked to the open report is the more useful state for anyone who picks either of them up. Nothing is requested of maintainers by this reopen.

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.

[Bug]: a typo in lakeview.model crashes config loading with a bare KeyError

1 participant