Skip to content

feat(engine): expose route output budget to hosts - #150

Merged
asto18089 merged 2 commits into
pinvou3-cleanfrom
feat/route-output-budget-exposure
Nov 10, 2026
Merged

asto18089 merged 2 commits into
pinvou3-cleanfrom
feat/route-output-budget-exposure

Conversation

@asto18089

@asto18089 asto18089 commented Nov 6, 2026 •

Copy link
Copy Markdown
Collaborator

Why

No-Issue: fork-only host-facing API exposure; no issue tracks it (repository has issues disabled).

Hosts that issue their own one-shot model requests must put the same max_tokens on the wire as the main turn. The route-effective cap (explicit operator override → documented ceiling ∩ automatic per-request API cap → compatibility/route ceilings → route window clamp, with the wire model normalized exactly as the transport does) lives in route_budget::effective_max_output_tokens_for_route, which is pub(crate) — so the host cannot reuse it and is pushed toward hard-coded constants that drift.

Concrete drift that motivated this (Pinvou review summon, 2026-11-06): the review call hard-coded max_tokens: 1600; the Kimi K3 reasoning model spent the entire budget on reasoning tokens (empty content, finish_reason=length), and even an interim hard-code of 8192 was 8× under the main-turn wire value (65536 = documented 131072 clamped by the 65536 per-request API cap) for the same route.

What

  • New public wrapper core::engine::route_effective_max_output_tokens(provider, base_url, requested_model, route_limits) next to the existing precedent context_input_budget_for_route (published for exactly this reason on the input side).
  • Re-exported from core::engine alongside it. No behavior change; the wrapper delegates to the single existing source.

Usage (host side, Pinvou/pinvou-agent)

deepseek_tui::core::engine::route_effective_max_output_tokens(
    provider, base_url, model, route_limits,
)

with route_limits = the effective limits the engine session resolved for the route (the same host-declared facts carried into the engine — not None, which falls back to catalogue defaults and diverges wherever the route carries declared facts), and provider/base_url from the same config the engine's client uses.

Notes

  • Fork-local exposure, not an upstream gap: upstream already ships route_budget as a pub mod of its engine crate (pub fn effective_max_output_tokens_for_route), so integrations there reach the math directly. The fork's single-crate layout keeps that module pub(crate), and this wrapper restores the same host-facing reachability next to context_input_budget_for_route. The parent-side consumer lands via the parent gitlink bump.
  • DCO: commit is signed off.

@asto18089
asto18089 changed the base branch from main to pinvou3-clean November 6, 2026 13:03
@asto18089
asto18089 force-pushed the feat/route-output-budget-exposure branch 3 times, most recently from d0322fd to c997d2e Compare November 6, 2026 14:43
@asto18089

Copy link
Copy Markdown
Collaborator Author

Fresh from-scratch review (6 parallel lanes + full local gate replication)

Verdict: approve-grade. The change is correct, minimal, and genuinely useful. Head c997d2ec0; all required checks green (check/Gitleaks/gate/DCO/link), merge state CLEAN.

What was verified (from zero, not from the PR's own claims):

  • Parity is real. The wrapper is a line-for-line extract of the transport clamp (client.rs::effective_max_output_tokens_with_limits, client.rs:2338-2351: wire_model_for_provider_route → effective_max_output_tokens_for_route). The main turn sources its request cap from the same math (turn_loop.rs:2285), and session.model is installed as the wire id at resolution time (route_runtime.rs:848), so no main-turn route (streaming/non-streaming/retry, any dialect, any reasoning shaper) computes a different number. Flagship value verified end-to-end: kimi-k3 documented 131_072 ∩ per-request cap 65_536 → 65_536 (route_budget.rs:163), matching the incident report.
  • Design follows the precedent exactly (context_input_budget_for_route, published upstream as feat(engine): expose context_input_budget_for_route for external budget reuse codewhale-hq/Codewhale#3968): same file, same pub use pattern, minimal surface; route_budget.rs is a private module so a bridge-visible wrapper is structurally required, and no pre-existing public path exposes the output cap (checked client/llm_client/ModelClient instance methods — all require a live client handle and re-resolve limits internally).
  • Smuggling sweep: clean. Single commit, single parent = base tip; tree delta identical at commit/parent/branch-comparison levels; only 3 em-dashes of non-ASCII; no whitespace/unicode tricks, no config changes, no drive-by edits; DCO correct.

Fixed during review (three force-pushes: 5c093b5d4 → 0167ee3a1 → d0322fd0e → c997d2ec0, all amended into the single commit, DCO preserved):

  1. REQUIRED — rustfmt was red on the original head: the new pub use sat after the #[cfg(test)] use, breaking rustfmt's ordering within the use group; cargo fmt --all -- --check failed, so fork-ci's check job would have been red. Reordered. (This had never surfaced because fork-ci never ran — the PR was originally opened against main, a disjoint upstream-mirror lineage; base switched to pinvou3-clean.)
  2. SHOULD — pin test added (route_effective_max_output_tokens_matches_the_transport_composition): k3 = 65_536 through the new public path, composition identity with the transport, DeepSeek alias convergence. Mutation-checked: swapping the wrapper's base_url/model args flips the pin to 8_192 (red). Note the honest limit: deleting the normalization line is not observable through budget values on today's catalogue (Moonshot passes ids through verbatim; DeepSeek family heuristics equalize deepseek-v4pro/deepseek-v4-pro — both probed empirically), so normalization is exactness-by-construction, not value-changing today; the test pins the composition for when a catalogue row makes spelling load-bearing.
  3. SHOULD — doc precision: the cap chain omitted the per-request output cap (the very rung that produces 65_536), the uncatalogued+route-cap replacement branch, the input contract (pass the engine-resolved RouteLimits; None diverges wherever routes carry declared facts), ambient env overrides, and per-dialect field shape (Codex Responses omits the field; some OpenAI reasoning routes rename it max_completion_tokens).
  4. REQUIRED — Gitleaks: my own doc edit ("per-request API cap,") tripped generic-api-key; reworded to "output cap" (verified locally with the exact gitleaks 8.30.1 + repo config: zero source findings).
  5. Admin — PR body: added the required No-Issue: line (the link gate was red), corrected the "upstream-first candidate" claim — upstream already ships route_budget as a pub mod with pub fn effective_max_output_tokens_for_route, so this is fork-local restoration of visibility (the fork's single-crate layout keeps the module pub(crate)), not an upstream gap — and stated the effective-limits input contract in Usage.

Not this PR's problem, but recorded: the motivating parent-side consumer does not exist yet in Pinvou/pinvou-agent (no main_wire_max_output_tokens; review/mod.rs still hard-codes 1600 in both one-shot branches and a test asserts it; the uncommitted parent gitlink bump points at the pre-review head and needs rebumping). Until that lands, this API has no caller — the fork half is done, the host half remains. cargo-audit/cargo-deny (advisories) red is pre-existing on pinvou3-clean (RUSTSEC-2026-0430).

Non-blocking follow-ups: turn_loop.rs:2285 computes the main-turn cap from session.model without normalization while the client path normalizes — parity holds today because the installed model is already the wire id, but unifying the two would close the latent drift lane; the sibling context_input_budget_for_route documents neither its expected model form nor its RouteLimits contract.

Local verification: fmt clean; cargo clippy --workspace --all-features --locked (CI shape) clean; wrapper pin + direct_moonshot_k3_route_uses_documented_1m_limits_with_provenance green.

@asto18089

Copy link
Copy Markdown
Collaborator Author

Fresh from-scratch re-review — round 2 (9 lanes + local gate replication + mutation battery)

Head reviewed: c997d2ec0. Rebase: no-op — the head sits exactly on the current pinvou3-clean tip (11cba403d = merge-base), nothing to re-apply. All 11 checks green on this exact head; the check job log confirms the new test ran and passed inside CI (PASS codewhale-tui core::engine::context::tests::route_effective_max_output_tokens_matches_the_transport_composition, workspace run 4322/15003).

Verdict: approve-grade; 0 BLOCKER / 0 MAJOR. The four review questions, answered from zero (not from the PR's own claims or the previous round):

1. Useful, complete, root-caused — yes.

  • Upstream story re-verified against a fresh upstream clone (codewhale-hq/Codewhale @ 1253aec03): crates/engine/src/lib.rs:53 is pub mod route_budget; with pub fn effective_max_output_tokens_for_route (route_budget.rs:356) — hosts there reach the math directly. The context_input_budget_for_route precedent is upstream-origin (PR feat(engine): expose context_input_budget_for_route for external budget reuse codewhale-hq/Codewhale#3968, merged 2026-07-03) and already present at the fork's v0.9.12 merge-base. The fork's effective_max_output_tokens_for_route is verbatim v0.9.12 — zero fork-local edits to the max_tokens rungs, so the wrapper exposes exactly the engine's own math.
  • Parity re-proven end-to-end: single wire choke point (prepare_outbound_request → clamp min(request.max_tokens, effective_max_output_tokens_with_limits(...)), client.rs:2158); the wrapper is a line-for-line extract of that clamp (client.rs:2339-2351); the main turn sources max_tokens from the same triple (turn_loop.rs:2285) after the route install has made session.model the wire id (route_runtime.rs:845 → engine.rs:5879-5891). All dialect builders serialise the clamped value (with documented renames/omissions); retries re-send the same body; context-recovery retries recompute the identical triple.
  • The motivating consumer now exists: parent PR Clipboard copy fails over SSH — 鼠标选中文本后自动复制在 SSH 下失败 (Copy failed) codewhale-hq/Codewhale#841 (fix(review): align pin/wu review budget with engine) calls the exact documented 4-arg signature (bridge.rs:2125-2128 → review/mod.rs:543), is sequenced after this PR, MERGEABLE with only its by-design gitlink-reachability gate red. The body's "consumer lands via the parent gitlink bump" is a fair description of a prepared landing, not vaporware.

2. Elegant — yes. Follows the precedent precisely (same file, same pub use seam, same doc rhythm); the wrapper's distinct name was forced (a direct pub use of effective_max_output_tokens_for_route collides with engine.rs's existing private import — E0252); the pub mod route_budget alternative would expose a wire-model-bound fn and push normalization onto every host (the wrapper bundles it); the instance-method alternative is unreachable (mod client is private). Option<usize> vs bare u32 asymmetry vs the precedent is honest — the output cap is infallible (.max(1)).

3. Smuggling — clean. Single commit, single parent == base tip; +72/−0 in exactly 2 files = doc + pub use + wrapper + 1 test; tree delta identical at commit/parent/branch level; only non-ASCII is 3 em-dashes consistent with the file's existing style; no whitespace/mode/config/workflow changes; DCO trailer matches author; commit form matches fork practice; changelog correctly untouched.

4. Bug-free — yes, production code. The whole chain re-audited function by function: total (no panics/unwraps/indexing reachable from host input), saturating arithmetic, fail-closed (uncatalogued → Uncatalogued(8_192) compat min kills any coincidental 65_536; the k3 pin is provably non-vacuous — losing the catalogue row flips it to 8_192); the test's DeepSeek alias pair lands on the catalogued Documented(384_000) row, not the fallback (not vacuously equal); per-call cost trivial (≤2 env reads, short read locks, first-call-only catalogue init). Env overrides: exact names confirmed in-chain (route_budget.rs:121/130), override-not-intersect semantics, sane on 0/negative/huge/blank values, and the invalid-CODEWHALE-does-not-fall-through-to-DEEPSEEK behavior is intentional and tested.

New non-blocking findings (this round)

  • MINOR — test env-seal gap: the new pin takes lock_test_env() but omits the EnvVarGuard::remove("CODEWHALE_MAX_OUTPUT_TOKENS") / ("DEEPSEEK_MAX_OUTPUT_TOKENS") pair that every sibling exact-cap pin applies — the hazard is documented in-code at route_budget.rs:678-689. Under an ambient override on a dev machine the numeric pin shifts and fails (CI unaffected: CI sets neither var and nextest isolates per-test). One-line fix matching sibling discipline. (Dynamically confirmed: with CODEWHALE_MAX_OUTPUT_TOKENS=16384 the test goes red.)
  • MINOR — DeepSeek alias leg doesn't guard normalization: both spellings reach Documented(384_000) through three normalization-independent layers (provider_capability alias arm config.rs:813, models.rs:347-349, models.rs:179-184), so deleting wire_model_for_provider_route from the wrapper leaves the suite green (dynamically confirmed). The kimi-k3 numeric pin is the load-bearing half (arg-swap mutation → 8_192, red). The in-test comment ("must reach the same cap") states a true invariant but overstates what the assertion verifies — worth a one-line comment correction or a genuinely normalization-sensitive id (e.g. an aggregator alias that only resolves after canonical rewriting).
  • SHOULD — no structural anti-drift pin: nothing cross-pins the wrapper to the transport clamp — both are independent copies of normalize→intersect, so a future edit to client.rs::effective_max_output_tokens_with_limits would silently drift from the host wrapper (the exact failure mode this PR exists to prevent). Cleanest fix: have effective_max_output_tokens_with_limits delegate to the wrapper (2-line-for-2-line swap, behavior-identical); alternatively a client-side parity test.
  • SHOULD — Some(RouteLimits) branch untested through the wrapper, though it is the doc's primary instructed usage; today only the None catalogue-default path is pinned (the Some path is covered one layer down, in route_runtime.rs:1273-1281).
  • MINOR — #[must_use] missing: both layers beneath the wrapper carry it (route_budget.rs:290, client.rs:2316/2338); the precedent lacks it too — cheap add, no lint impact.
  • NOTE — doc precision: the "route window" rung is actually window − 2_048 plus a .max(1) floor (override 384_000 on a 327_680 window → 325_632); the doc never says the base_url must be the one the request will dispatch on (custom endpoints skip normalization entirely); the max_completion_tokens rename set is wider than "some OpenAI reasoning routes" (also XiaomiMimo, MiniMax M3, and the direct Moonshot k3 route); the PR body's "single-crate layout" is really "engine lives inside the tui crate" (the workspace is multi-crate), and "keeps that module pub(crate)" — the module is a plain private mod, its items are pub(crate).
  • NOTE — upstream vintage: the wrapper exposes the fork's v0.9.12-era chain; upstream main has advanced (RouteLimits.output_declared + declared_route_override, the Compaction: 15 of 16 recorded compactions failed, and the summary call reads ~219k tokens with 0% cache hit codewhale-hq/Codewhale#6540 window-relative fallback, reasoning-effort-aware effective_max_output_tokens_for_turn). Host values follow automatically at the next sync — that is the wrapper doing its job — but the current output vintage is v0.9.12.
  • Parent coordination (for the feat(engine): expose route output budget to hosts #150 → Clipboard copy fails over SSH — 鼠标选中文本后自动复制在 SSH 下失败 (Copy failed) codewhale-hq/Codewhale#841 sequence, not this PR's code): Clipboard copy fails over SSH — 鼠标选中文本后自动复制在 SSH 下失败 (Copy failed) codewhale-hq/Codewhale#841 currently pins gitlink 5c093b5d4 and its body names d0322fd0e as the feat(engine): expose route output budget to hosts #150 head — after this PR merges, Clipboard copy fails over SSH — 鼠标选中文本后自动复制在 SSH 下失败 (Copy failed) codewhale-hq/Codewhale#841 must re-pin to the advanced pinjou3-clean head before its gate can go green (docs/fork-modifications.md:406 forbids pinning pre-merge wave SHAs). Also visible at Clipboard copy fails over SSH — 鼠标选中文本后自动复制在 SSH 下失败 (Copy failed) codewhale-hq/Codewhale#841: memory/llm_review.rs:722 (900) and memory/organize.rs:1258 (1500) share this PR's exact motivating failure mode and are neither fixed nor disclosed there.

Local replication (this machine, fresh clone at c997d2e)

  • cargo fmt --all -- --check: clean (the format break found in round 1 stays fixed).
  • cargo clippy --workspace --all-features --locked in fork-ci's exact shape (with the repo's lint allowlist): clean. (Without CI's allowlist flags, strict -D warnings reports 8 pre-existing errors — 6 too_many_arguments, 2 collapsible_if — both categories CI explicitly allows; nothing new from this PR.)
  • Targeted: new wrapper pin green (1 passed; 12427 filtered out); direct_moonshot_k3_route_uses_documented_1m_limits_with_provenance green; uncatalogued-ceiling pin green.
  • Env-seal proof: CODEWHALE_MAX_OUTPUT_TOKENS=16384 → new pin fails (left: 16384, right: 65536 at context.rs:1077); unset → green again. Confirms the MINOR above.
  • Mutation M-A (swap wrapper base_url/model args): red — pin drops exactly to the 8_192 uncatalogued ceiling; the test catches argument-order regression.
  • Mutation M-B (delete the wrapper's normalization call; compile-clean mutant): green — mutant survives, dynamically confirming the normalization blind spot above.
  • gitleaks dir . from repo root (exact CI version 8.30.1 + repo config): 0 findings.

None of the above blocks merge; MINORs 1-2 and the two SHOULDs are one-commit follow-ups that can ride the next force-push or a later pass. The #150 → codewhale-hq#841 re-pin is mandatory sequencing, already anticipated by codewhale-hq#841's own body.

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

已审查固定 head c997d2ec0d0f02b56024fa8040d6faf2bd8d6da0,base 11cba403dc6bd6ca5c4dca55042785b03449ac8e,批准本 PR。

Standards

完整两文件 +72 行差异未发现阻断问题。公开包装及 re-export 沿用既有输入预算接口的边界;包装函数与客户端使用同样的 provider/base_url 模型归一化,再调用同一现有路由预算函数。没有新增第二套限额算法或修改现有请求行为。

非阻断 P3:新测试虽持有环境锁,但没有临时移除并恢复 CODEWHALE_MAX_OUTPUT_TOKENS / DEEPSEEK_MAX_OUTPUT_TOKENS;合法继承覆盖会改变 65,536 的固定断言。建议沿用邻近测试的 EnvVarGuard 方式隔离,并补充 Some(RouteLimits) 控制。当前组合断言与别名对照也不足以单独检测未来客户端流程漂移。

Spec

公开签名、effective RouteLimits 输入契约、环境覆盖及传输字段差异说明符合本次“暴露既有预算计算”的范围。已核对 route 的 wire-model 安装、主循环 active limits、客户端最终取较小请求值的路径。接口提供路由上限,仍由宿主传入同一配置及有效限额;未声称已经验收父仓消费者或 gitlink 集成。

实际验证:fetch 最新 base/head、完整差异/调用链/测试内容审查、public re-export 与客户端组合的源码核对、单提交/DCO 检查、git diff --check 通过;只读核对该 head 的 11 项检查均成功。本机未运行 Rust/Cargo、测试、环境覆盖实验或 provider 请求;作者报告的原生运行和变异计数不作为本轮执行结果。发送前重新核对 head/base/讨论,以及 auto-merge/队列均未启用。未执行合并相关操作。

Standards:无阻断问题,保留测试稳健性建议。Spec:本 PR 的 API 暴露范围通过,父仓集成另验。

@asto18089

Copy link
Copy Markdown
Collaborator Author

Thank you for the approval and the robustness note. The env-var P3 is acknowledged: the new test holds the environment lock but does not temporarily strip CODEWHALE_MAX_OUTPUT_TOKENS / DEEPSEEK_MAX_OUTPUT_TOKENS, so a legitimate inherited override would break the fixed 65,536 assertion, and the combo assertion plus alias cross-check indeed would not alone catch a future client-flow drift. Adopting the neighboring test's EnvVarGuard pattern plus a Some(RouteLimits) control is recorded as a bounded follow-up for the next revision on this branch — deferred now since the PR is approved at the current head and the change would re-open the test evidence chain mid-review.

@asto18089
asto18089 force-pushed the feat/route-output-budget-exposure branch from c997d2e to dc73de7 Compare November 7, 2026 12:41

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approve dc73de769b75fbc2843a66dad3ce4b1aeae0d6d0, independently checked out against fetched maintenance 11cba403dc6bd6ca5c4dca55042785b03449ac8e.

Standards

The ambient-environment P3 is closed. Both output-token overrides are removed by restoring guards after acquiring the existing test lock; reverse drop order restores their original values before unlocking, including assertion unwinding. The two changelog entries accurately describe the existing public exposure. Production context code and the engine re-export are unchanged from the previously reviewed head; current author/trailer identity matches.

Spec

The wrapper still normalizes the wire model and delegates to the existing route budget calculation. The test-only isolation change preserves that behavior. The nonblocking suggestion for a Some(RouteLimits) control and an independent client-flow comparison remains useful future drift coverage; no current composition defect is established.

Validation: cumulative diff check, exact production/re-export identity and guard-lifetime/source inspection. No local Cargo, native environment experiment, provider execution or author test-count adoption. This approval covers this API exposure, not an unreviewed parent consumer or gitlink update. Required CI remains unwaived; no merge or queue operation.

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

补充固定 head dc73de769b75fbc2843a66dad3ce4b1aeae0d6d0 的规范反馈;已有源码批准 5812347923 的功能结论保留。

Standards

环境隔离 P3 已在源码层面闭合:两项 EnvVarGuard 在环境锁之后声明,退出/断言 unwind 时先恢复原值,再释放锁。已核对生产 context 部分及 engine re-export 与此前已审 head 逐字节相同。

[P3] 请移除本 PR 新增的两份 changelog hunk。 CONTRIBUTING.md:191–197 明确要求 changelog 在 main 合并时由发布管理者统一编写,并明确禁止在 PR 分支修改 CHANGELOG.md 和 crates/tui/CHANGELOG.md。本次两处新增内容虽然描述准确,仍与该约定冲突;本轮没有看到适用例外。请仅剥离这两处新增 hunk,保留环境隔离修复,或由维护者明确说明例外。此项是范围有限的贡献规范清理,不新增生产正确性 P2。

Spec

API 暴露行为未变化,本轮需求审查通过。Some(RouteLimits) 控制及独立客户端流程一致性测试仍为有用的非阻断后续覆盖,尚未发现当前组合错误。父仓消费者及 gitlink 集成仍需另验。

实际验证:完整最新增量、守卫捕获/恢复及生命周期源码、生产代码字节一致性、累计 diff check;未运行本机 Rust/Cargo、真实环境变量实验或 provider。作者/其他审核者的原生运行计数未作为本轮执行结果。发送前已重核 head/base/讨论。

Hosts issuing their own one-shot requests (the Pinvou review summon)
must put the same max_tokens on the wire as the main turn. Reusing the
route-effective cap via a public wrapper mirrors the
context_input_budget_for_route precedent: share the math instead of
re-deriving constants and silently drifting from the engine.

Signed-off-by: asto <asto18089@126.com>
@asto18089
asto18089 force-pushed the feat/route-output-budget-exposure branch from dc73de7 to af494b7 Compare November 7, 2026 13:21
@asto18089

Copy link
Copy Markdown
Collaborator Author

Accepted — the two changelog hunks are stripped on the rewritten head af494b7a8. CONTRIBUTING.md:191–197 indeed reserves changelog writing for the release manager at merge time, and no exception applies here: both CHANGELOG.md and crates/tui/CHANGELOG.md are restored to the base state (the rewrite is the single PR commit amended — the cumulative delta versus the previously reviewed head is exactly the 8+8 changelog-line removal, production and test content byte-identical, author/trailer identity unchanged). The environment-isolation EnvVarGuard fix and everything else in the PR stays untouched.

@JensenChen28 JensenChen28 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

批准固定 head af494b7a81c86e5a99b769da5559dcf30e9a2c88,base 11cba403dc6bd6ca5c4dca55042785b03449ac8e。

Standards

两份 changelog 均已恢复为 base 的原始字节,关闭此前 CONTRIBUTING.md:191–197 的规范反馈。与已审 dc73 head 相比仅移除这 16 行;生产代码、测试、公开签名及 engine re-export 保持一致,环境变量隔离修复保留。当前 author/Signed-off-by 匹配。

Spec

公开预算接口及其模型归一化、现有限额计算委托符合范围,无新增问题。Some(RouteLimits) 控制和独立客户端流程一致性覆盖仍可作为非阻断后续工作;父仓消费者与 gitlink 集成尚未验收。

实际验证:重新 fetch、完整最新差异及累计范围复核、Git blob/字节一致性、守卫修复保留和 diff check。未本机运行 Cargo/Rust、环境变量实验或 provider;未借用外部测试计数。发送前核对 head/base/讨论,auto-merge 与队列均为空。未执行合并相关操作。

@zhuowp zhuowp left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved at af494b7a81c86e5a99b769da5559dcf30e9a2c88 against 11cba403dc6bd6ca5c4dca55042785b03449ac8e. I reviewed both files and all three changed fragments. The public helper uses the same model normalization and numeric ceiling as the client when given the same resolved provider, endpoint, model and limits. Request serialization still determines the field for each dialect.

The fixed Linux CI tree passed the new regression and recorded 15,003 tests passed with 15 skipped. All 11 checks succeeded. This approval covers the library API. Downstream host adoption and live provider behavior require separate validation.

@asto18089
asto18089 merged commit e972a9a into pinvou3-clean Nov 10, 2026
11 checks passed
asto18089 added a commit that referenced this pull request Nov 11, 2026
Merges pinvou3-clean@6f7a3a748 (#136/#142/#148/#150/#152) into the
rate-limit recovery branch, preserving both histories per the
established integration pattern.

Conflict resolutions (composed, not one-sided):
- client.rs: keep the deferred `prepare_outbound_request` fallback from
  the maintenance route-budget work and wrap the prepared-stream
  operation in `rate_limit::with_operation` so recovery still owns the
  logical operation regardless of preparation timing.
- turn_loop.rs: adopt the maintenance-typed `TurnFailure` +
  `split_stream_open_error` plumbing while keeping the rate-limit
  interruption contract: a RateLimited/QuotaWindow failure parks the
  turn as `Interrupted` (message surfaced, no terminal error source
  consumed) and otherwise `finish()` records the typed source as
  before. Both stream-open sites classify first, then flag
  `rate_limit_interrupted`.

Signed-off-by: asto18089 <asto18089@126.com>
@asto18089

Copy link
Copy Markdown
Collaborator Author

The follow-ups recorded in this thread (the Some(RouteLimits) control promised in the 2026-11-07 acknowledgment, plus the remaining review findings) are now landed in #155: the normalize-then-intersect composition is a single shared route_budget copy that both the transport clamp and this wrapper delegate to (closing the client-flow drift lane structurally), the wrapper test gains the limits control and a genuinely normalization-sensitive OpenRouter kimi-code pin (32,768 canonicalized vs 8,192 raw — mutation-verified red), and the wrapper doc/​#[must_use] precision items are closed. The EnvVarGuard half had already landed pre-merge at dc73de769.

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.

3 participants