0.9.12: perf fold, quieter chrome, compatible hosts, delete staged runtime_contract - #5667
Conversation
strace census on the doctor path showed 88.7% of syscall time in futex with 45 clone3 spawns: build_runtime() built a full multi-thread runtime (one worker per CPU, 16MiB stacks each) for a read-only diagnostic that uses none of it. Cap workers at 2 for Commands::Doctor only; interactive sessions and servers keep default sizing. Verified on spark-1672 (aarch64): clone3 45->27, futex calls 265->99, doctor wall 45.4->42.8ms, --help output byte-identical, --version floor unchanged.
bundled_models_dev_catalog() re-parsed the ~50KB snapshot at every call site: the client route path, provider picker, provider lake, and fleet identity each paid an independent serde parse. Return a &'static catalog backed by OnceLock — the asset is include_str! constant, so sharing the parsed form is immutability-safe. Doctor wall is unchanged (the init path only parses once either way); this removes redundant parses on multi-call paths.
…'s rustc spawns RustC::resolve() proved presence by executing 'rustc --version' and discarding stdout; doctor's rustc_version() then launched a second rustc process to read the same banner. Each launch loads libLLVM. probe_executable_capturing() now records the banner during the probe (OnceLock), and the diagnostics path consumes it after an available() check. Verified on spark-1672 (aarch64): execve(rustc) 2->1 per run, doctor wall 42.8->37.6ms (-12%, n=60), 'rust:' line byte-identical to the toolchain's own 'rustc --version', help output unchanged.
codewhale eval is a sequential offline tool-loop; it needs no async concurrency, so build its runtime with the same 2-worker cap doctor uses instead of one worker per CPU. Interleaved A/B on spark-1672 under load 9-11: eval wall -8/-10/-6 percent across three OLD/NEW batch pairs; thread spawns for the command drop from ~45 to 5. Interactive surfaces unchanged. style(tui): rustfmt the runtime builder chain
setup --status, sessions listing, and session diagnostics share the doctor dispatch shape: read-only, short-lived, no async concurrency need. Extend diagnostic_worker_count to cover them via a structured match mirroring telemetry_command_is_read_only. Interleaved A/B on spark-1672 for 'setup --status' (-17/-21/-16 percent across three batch pairs, NEW wins all three); interactive and mutating surfaces unchanged.
…ercent The persisted models.dev cache stores a ~5MB catalog body JSON-escaped inside a JSON envelope. maybe_load_persisted_cache() runs synchronously on the interactive boot path and parsed the body twice (envelope, then re-parse of the escaped body) plus a full-body string copy — a gdb mid-boot sample caught serde_json::visit_map inside models_dev_live during the largest silent window of startup, and strace showed a 17.7ms zero-syscall compute burst after config load. v2 format: one-line JSON metadata header followed by the verbatim catalog body. Loading does one small header parse, one body parse, zero body copies; v1 envelopes still load via fallback and every refresh now writes v2. Measured on spark-1672 (aarch64), identical binary, interleaved batches over scratch CODEWHALE_HOMEs differing only in cache format: time-to-first-frame median 44.0/52.7/48.6/51.7ms (v1) vs 13.9/14.1/13.9/14.4ms (v2).
apply_provider_model_cutlines called ApiProvider::parse per offering; parse scans every provider and its alias list with case-insensitive compares, and the live models.dev snapshot carries thousands of rows. Freeze-and-inspect at t+180ms of boot caught the main thread inside this loop. Resolve each distinct provider string once through a HashMap. Interleaved A/B on spark-1672: first-paint median drops ~2-6ms per run (195.7/196.9/192.9/197.2 -> 185.0/190.9/190.3/193.8, NEW wins 4/4). perf(tui): adaptive poll cadence for foreground shell completion Foreground bash runs go through execute_foreground_via_background, whose wait loop polled child status on a fixed 100ms tick. A command that finished in 2ms was only noticed at the next tick, so every fast foreground call (ls, grep, wc, echo — the bulk of agent traffic) carried a ~100ms floor: a simulated 8-tool turn spent ~400ms of its ~526ms wall in poll quantization (measured over serve --mcp with the real registry). Replace the fixed tick in all three wait loops (foreground completion, wait-many, delta waiter) with an adaptive cadence: first sleep 10ms, then double to a 100ms cap. Instant commands are now detected within ~10ms; long-running commands reach the old cap after one doubling step, so their overhead is unchanged. Revert "perf(tui): adaptive poll cadence for foreground shell completion" This reverts commit 1a2f42c9f0b53d9b83ed50db4d4d7bd66df7977e.
Foreground bash runs go through execute_foreground_via_background, whose wait loop polled child status on a fixed 100ms tick. A command that finished in 2ms was only noticed at the next tick, so every fast foreground call (ls, grep, wc, echo — the bulk of agent traffic) carried a ~100ms floor: a simulated 8-tool turn spent ~400ms of its ~526ms wall in poll quantization (measured over serve --mcp with the real registry). Replace the fixed tick in all three wait loops (foreground completion, wait-many, delta waiter) with an adaptive cadence: first sleep 10ms, then double to a 100ms cap. Instant commands are now detected within ~10ms; long-running commands reach the old cap after one doubling step, so their overhead is unchanged. Measured over real serve --mcp (n=150, binaries aside): bash `true`: p50 13.6 both | p95 16.2→13.9 | p99 21.5→18.6 | p100 35.9→21.6 bash grep: p99-p100 flat ~13.8-14.1 both file_read: p100 0.2 both (min/p50/p99 all 0.1) tools/list: p100 0.2-0.7 both Tail-wall win: p95 -14%, p99 -13%, p100 -40% with p50 unchanged. An earlier A/B that compared only medians missed the win and was reverted; the tail distribution above is what justified re-landing it. Interleaved A/B on aarch64 Linux, same host, same window.
The per-turn metadata build and the compaction decision each re-walked the full transcript more than once per step. Make those walks single-pass with byte-identical behavior, and fix one cache-invalidation bug found along the way. compaction_decision_with_billed: the same pure estimator ran twice per step once pressure existed, once in the pressure gate and once in the prune projection. Compute it at most once and thread it to both consumers. Guard order (gate, prune projection, too-few-messages, retained-floor) is untouched and decisions are identical for all inputs. When provider-billed input alone proves pressure, the estimate walk is skipped entirely without bypassing the guards that keep a compaction loop from re-firing on every step. active_input_tokens_with_current_text: the per-turn metadata builder deep-cloned the entire message history just to append the composer text before estimating. Estimate the history in place and add the one-message delta arithmetically instead. The estimator inflates the summed byte count by a factor of 1.5 rounded up as a whole, so the delta is floor(own * 3 / 2) plus one exactly when the running byte sum is even and the composer text contributes an odd count. Proven against a clone-and-estimate reference over 80,000 parity combinations and pinned per case by a test that also covers replayed reasoning, the arm where a naive helper undercounts. Op::SyncSession: session restore assigned the restored message list directly to the session field, bypassing the messages_revision bump the token-estimate cache keys on, so a restored conversation could be priced from a stale estimate. Bump the revision and pin the behavior with a test. Verification: cargo fmt clean; compaction:: and engine preview, pressure, and restore tests 19 passed via remote rch lane; cargo check --all-targets clean with no warnings.
The streaming render ran the math transform over the full accumulated content on every chunk, before the incremental markdown cache that itself only renders deltas, so streamed turns paid an O(n) copy and scan per chunk with O(n^2) cumulative cost. render_latex_in_text now returns Cow<str>. One byte scan for the three opening delimiters ($, \(, \[) decides between borrowing the input untouched, the overwhelming case for streamed prose, and running the transform. Output is byte-identical either way: the full transform still re-verifies delimiters precisely, so the fast scan cannot create false negatives. Tests: no_math_is_borrowed_without_copy pins the borrowed path and that the \( form is not missed by the fast scan; test_inline_dollar pins the owned path on math input. Verification: cargo fmt clean; latex_render:: 18 passed via remote rch lane; cargo check --all-targets clean with no warnings.
Convert the first audited test-only dead-code slice to cfg(test), covering rendering/text fixtures and helper surfaces while preserving runtime behavior. Add direct tests for scroll and exploration wrappers for #5587.
…e_contract Hosted Chat Completions backends (Baseten, Groq, Cerebras) land as Compatible setup templates, not ProviderKind variants. MCP manager copy names the server, the failure, and one recovery command. Idle metrics omit zeros. Working chrome says "in the current". Nine uncompiled runtime_contract files are gone. Dead-code budget 448 -> 372.
|
Claude finished @Hmbown's task in 4m 45s —— View job Reviewing PR #5667
SummaryReviewed the diff against 🐛 Correctness: MCP manager now shows a false "needs attention" warning for healthy, connected servers
lines.push(format!(
" {}",
crate::mcp::mcp_startup_warning(&server.name, recovery, server.error.is_some())
));
The new _ => format!("The {name} MCP server needs attention. Run `{command}`."),So a fully healthy, connected MCP server now unconditionally prints "The Suggested fix: only push the warning line when the server actually needs attention (e.g. 🧹 Reuse/simplification: duplicate full-transcript pass undercuts the "single-pass" perf goal
let base = estimate_input_tokens_conservative(&self.session.messages, system_prompt); // pass 1
...
let sum: usize = self.session.messages.iter().map(|m| estimate_tokens_for_message(...)).sum(); // pass 2, same valuesThis isn't a correctness bug (verified the parity-based delta formula — Provider/model/route safety (EPIC #2608)
Other notes (no issues found)
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b051ee7a37
ℹ️ 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".
| /// / `MCP startup incomplete (failed: X)`). | ||
| #[must_use] | ||
| pub fn mcp_startup_warning(name: &str, kind: McpRecoveryKind, failed: bool) -> String { | ||
| let command = kind.slash_command(name); |
There was a problem hiding this comment.
When a configured MCP server name contains whitespace, this helper still interpolates it into /mcp enable or /mcp login. The surrounding push_server code deliberately falls back to /mcp validate when mcp_name_is_command_safe rejects the name, but the new warning bypasses that result; for example, /mcp login github mcp is parsed as a login for github, so the suggested recovery targets the wrong server. Pass the already-sanitized command into this helper or apply the same safety check here.
Useful? React with 👍 / 👎.
| let command = kind.slash_command(name); | ||
| match kind { | ||
| McpRecoveryKind::Reauth => { | ||
| format!("The {name} MCP server requires OAuth reauthentication. Run `{command}`.") |
There was a problem hiding this comment.
When the MCP manager is rendered under any non-English locale, this newly added warning remains English because the helper accepts no locale and formats literal prose; push_server only applies its locale to surrounding metadata. Add typed MessageId entries and render the warning through tr so localized users do not receive mixed-language recovery instructions.
AGENTS.md reference: crates/tui/AGENTS.md:L22-L23
Useful? React with 👍 / 👎.
| "PhaseWorking": "in the current", | ||
| "PhaseReasoning": "reasoning", | ||
| "PhaseReading": "reading", | ||
| "PhaseUsingTool": "using tool", | ||
| "PhaseSubagents": "working on subagents", | ||
| "PhaseSubagents": "pod underway", |
There was a problem hiding this comment.
When any complete non-English locale is selected, these phase keys still resolve to their old translations because only en.json was changed; every sibling pack retains the previous working/subagent labels. Update the complete packs alongside these English changes so the new in the current and pod underway terminology is consistently represented.
AGENTS.md reference: crates/tui/locales/AGENTS.md:L7-L11
Useful? React with 👍 / 👎.
| pub fn bundled_models_dev_catalog() -> ModelsDevCatalog { | ||
| ModelsDevCatalog::parse_json(BUNDLED_MODELS_DEV_JSON) | ||
| .expect("committed bundled Models.dev asset must be valid JSON") | ||
| pub fn bundled_models_dev_catalog() -> &'static ModelsDevCatalog { |
There was a problem hiding this comment.
External users of the published codewhale-config crate that consume bundled_models_dev_catalog() as an owned ModelsDevCatalog will stop compiling because this performance change alters the public signature to &'static ModelsDevCatalog. Keep the existing owned-returning API and add an internal or additive borrowed cached accessor instead, so parse memoization does not introduce a source-breaking change in a patch release.
AGENTS.md reference: AGENTS.md:L15-L17
Useful? React with 👍 / 👎.
Add GLM-5.3-Flash as a first-class picker row (wire id GLM-5.3-Flash, OpenRouter z-ai/glm-5.3-flash) so /model can select it. Flash is the faster/explore sibling of GLM-5.3; the Z.ai default stays GLM-5.3. Ship the published $0.15/$0.50 list, not the 50% promo.
|
Claude finished @Hmbown's task in 2m 13s —— View job Reviewing PR #5667 (update)
SummaryThis synchronize event added one new commit since my last review: ✅ New commit (
|
Update the stale Z.ai fast-tier expectations to the newly registered GLM-5.3-Flash route, including its high-effort capability receipt. Remove three Clippy failures in the catalog and live-model paths, add the durable Signed-off-by: CodeWhale Bot <bot@codewhale.net> #5643/#5655 changelog receipt, and regenerate the packaged TUI changelog.
|
Claude finished @Hmbown's task in 1m 15s —— View job Reviewing PR #5667 (update)
SummaryThis synchronize event added one new commit since my last review: ✅ New commit (
|
3 participants
No-Issue: release-train consolidation for already-linked changes.
Summary
Folds Aditya's startup/token-accounting perf (#5664/#5665) and Isabel's #5587 test-only
#[cfg(test)]slice (#5666) onto currentmain, then:ProviderKindvariants. Same shape as SenseNova / OMP catalog rows.in the current.runtime_contract/staging files. Live contracts remainmodel.rsandtermination.rs.docs/design/TUI_DECONSTRUCTION.md(consolidate before extract; no micro-crates).Testing
python3 scripts/check-provider-registry.py./scripts/dev-test.sh config provider_templates::(8 passed)python3 scripts/check-dead-code-budget.py(372, locked)Do not merge red required CI. Do not merge #5576 / #5628.