Skip to content

fix: generate catalog when user configures context window (#1594) - #1786

Open
dongyu23 wants to merge 3 commits into
BigPizzaV3:mainfrom
dongyu23:fix/1722-catalog-context-window-tests
Open

fix: generate catalog when user configures context window (#1594)#1786
dongyu23 wants to merge 3 commits into
BigPizzaV3:mainfrom
dongyu23:fix/1722-catalog-context-window-tests

Conversation

@dongyu23

@dongyu23 dongyu23 commented Aug 5, 2026

Copy link
Copy Markdown

Summary

Completes PR #1722 by fixing the two failing integration tests.

PR #1722 fixed the early return in apply_model_catalog_to_config that silently dropped the Manager's context window for suffixless custom models. Without the fix, Codex fell back to its bundled 272000 default and the CLI showed ~258K instead of the configured value (#1594).

The fix moves the fallback parse before the early return and adds fallback.is_none() to the guard, so a user-configured context window forces catalog generation.

Test fixes

Two integration tests failed because they set context_window=200000 while asserting no catalog generation. With the fix, configuring a context window now correctly triggers generation.

  • apply_relay_profile_does_not_write_model_catalog_json_for_selected_models: removed context_window / auto_compact_limit, preserving the original intent — no catalog when nothing is customized.
  • apply_relay_profile_no_catalog_when_model_list_has_no_suffix → renamed to apply_relay_profile_generates_catalog_when_context_window_configured: now tests the positive case — context_window set, no suffix → catalog IS generated with the correct window value in both config.toml and the catalog JSON.

Verification

cargo fmt --all -- --check   ✅
cargo clippy -p codex-plus-core --all-targets   ✅ (zero warnings on changed code)
cargo test -p codex-plus-core --test relay_config   ✅ 107 passed, 0 failed
cargo test -p codex-plus-core --lib   ✅ 209 passed, 0 failed

Closes #1722
References #1594

…#1594)

PR BigPizzaV3#1722 fixed the early return in apply_model_catalog_to_config that
silently dropped the Manager's context window for suffixless custom
models. Codex then fell back to its bundled 272000 default and the CLI
showed ~258K instead of the configured value.

The fix moves the fallback parse before the early return and adds
fallback.is_none() to the guard, so a user-configured context window
forces catalog generation.

Two integration tests failed because they set context_window=200000
while asserting no catalog generation. Both tests now omit
context_window and auto_compact_limit, preserving their original intent:
no catalog when nothing is customized.

Closes BigPizzaV3#1722
References BigPizzaV3#1594
@BigPizzaV3

Copy link
Copy Markdown
Owner

代码审查:#1722 的失败测试已在本 PR 中按新行为修正,逻辑与 issue #1594 目标一致。当前没有可用的 CI check(状态 UNSTABLE),建议先触发/补齐全平台 CI,再合并。

@dongyu23

Copy link
Copy Markdown
Author

已补齐全平台 CI,并修复了 Windows checkout 下字节哈希测试受 CRLF 转换影响的问题。

最新提交:48163e9

Fork 上针对该提交手动触发的完整 PR build artifacts 已全部通过:

  • Windows artifacts:成功(包含前端测试、TypeScript 检查、前端构建、cargo test --workspace、release 构建、NSIS 安装包及产物上传)
  • macOS DMG (x64):成功
  • macOS DMG (arm64):成功

运行记录:
https://github.com/dongyu23/CodexPlusPlus/actions/runs/31497679527

上游 PR 页面目前仍未显示原生 checks;此前上游 run 的 action_requiredjobs=[] 表明外部 fork workflow 需要仓库维护者批准。烦请批准/触发上游 workflow,或参考上述同 SHA 的全平台绿色运行结果。

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.

2 participants