Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7ae783bff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| const TOKEN_BUDGET_REMINDER_MESSAGE_TEMPLATE_MAX_BYTES: usize = 4096; | ||
| const TOKEN_BUDGET_GUIDANCE_MESSAGE_MAX_BYTES: usize = 4096; |
There was a problem hiding this comment.
When token_budget is enabled and a config supplies a near-4096-byte ASCII reminder or guidance string, these constants allow that entire value to be injected as one developer-context fragment via TokenBudgetReminder or ContextWindowGuidance, so a single new context item can cross 1k tokens. The repo review rule requires any newly crossable >1k-token context item to be highlighted as P0 for manual review; keep the hard cap below that threshold or add token-based truncation before injection.
AGENTS.md reference: AGENTS.md:L91-L100
Useful? React with 👍 / 👎.
| #[schemars(length(min = 1, max = 4096))] | ||
| pub reminder_message_template: Option<String>, | ||
| /// Guidance appended to the context-window metadata in a developer message. | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| #[schemars(length(max = 1000))] | ||
| #[schemars(length(max = 4096))] |
There was a problem hiding this comment.
For non-ASCII token-budget messages in the newly allowed 1001–4096 character range, this generated maxLength says the config is valid, but resolve_token_budget_config still enforces the same value with String::len() bytes and rejects it once UTF-8 encoding exceeds 4096 bytes. That makes schema-validated configs fail at startup for values such as a 3000-character guidance string containing two-byte characters; either enforce character counts in the loader or keep the schema/documentation byte-based for this config-loading surface.
AGENTS.md reference: AGENTS.md:L102-L110
Useful? React with 👍 / 👎.
|
Discussed offline and decided 2,000 bytes is reasonable in this case. |
Why
Token-budget reminder and guidance messages can require more than 1,000 bytes to provide useful model-facing instructions. At the same time, these strings are injected into model-visible context, so their size must remain tightly bounded in response to the P0 context-growth concern. A 2,000-byte runtime cap provides additional room without allowing the substantially larger context growth of a 4 KiB limit.
What changed
maxLengthvalues to 2,000codex-rs/core/config.schema.jsonTesting
just test -p codex-featuresjust test -p codex-core load_config_resolves_token_budget_config load_config_rejects_invalid_token_budget_reminder_templateThe full
codex-coretest run completed 2,858 tests successfully and encountered seven unrelated environment-sensitive failures involving Seatbelt/network environment assertions, MCP capability setup, and abort timing.