[asto18089]

Problem

The Moonshot / Kimi tool-schema pipeline had drifted into shapes justified by live probes, per-route whitelist tiers, and model rosters rather than by the published specification. Concretely: the wire carried keywords the MFJS specification does not list; root-level unions were handled inconsistently against the object-root rule; Kimi Code's fixed-sampling strip was keyed to a model roster (missing the overseas host); resource budgets came from probe guesses instead of the reference validator; and digit-led tool names could violate the documented name pattern.

Document evidence (verbatim quotes, fetched 2026-11-05)

• MFJS specification — https://github.com/MoonshotAI/walle/blob/main/docs/mfjs-spec.zh.md (English original at docs/mfjs-spec.md in the same repository), fetched 2026-11-05. Closing clause, verbatim: 「若官方规范中的某些概念、关键字或特性在本文档中未被提及，则表示当前 MFJS 尚未提供官方支持……我们不保证它们能在最终的结构化输出中产生稳定、符合预期的结果」— anything not listed is unsupported, so the wire carries only the listed set. The default section, verbatim: 「default 字段为目标输出提供一个默认值……这儿不做类型检查，需要调用者自行保证其值类型内容和所在位置期望的内容相匹配」— default accepts any value on any node, with no type check.
• Moonshot tool-use documents — https://platform.kimi.ai/docs/api/tool-use, fetched 2026-11-05: "The root of parameters must be an object" — root-level unions stay flattened; and the function name pattern ^[a-zA-Z_][a-zA-Z0-9-_]{0,127}$ — a leading digit sits outside it.
• Kimi Code API hosts — https://www.kimi.com/code/docs/en/third-party-tools/codex.html ("Kimi Code API address — China https://api.kimi.com/coding/v1, overseas https://api.kimi.ai/coding/v1"), re-verified 2026-11-05: the fixed-sampling rule is a property of the membership endpoint, so both hosts must be covered, and the documents require the fixed sampling fields to be omitted.
• walle validator source — https://github.com/MoonshotAI/walle (conf.go, DefaultValidatorConfig), fetched 2026-11-05: the reference budgets (500 / 3000 / 30 / 1000 / 75000). Where documents and server source disagree, the documents govern (source comparison notes retained in-code).

Changes (20 commits, head bed78918d)

1. The wire whitelist narrows to the published MFJS set — type, description, default, properties, required, additionalProperties, items, enum, with $defs/$ref under their documented restrictions. The nine advisory keywords (title, $comment, $id, and the min/max bound series) are stripped in strip_mfjs_advisory_keywords upstream of validation, never shipped; every other unlisted keyword (e.g. pattern, format, $schema, prefixItems, patternProperties) fails closed with UnsupportedKeyword and the tool is dropped — pinned per keyword by probe_observed_keywords_fail_closed_on_every_route.
2. enum gains same-shape scalar validation; enum-carrying violations are stripped instead of shipped, and a malformed enum ([] or non-array) stays fail-closed in the validator — the strip never widens it (fixed and pinned in 6d37a52cf).
3. Root-level unions keep the flattening pass, per "The root of parameters must be an object". Every dropped branch-required group is now disclosed in the constraint note, including a single group (fixed and pinned in 6d37a52cf). Undocumented approximation retained: when branches define the same property differently, the first definition wins silently.
4. The Kimi Code fixed-sampling strip becomes route-scoped and covers the overseas api.kimi.ai host alongside api.kimi.com; 6/6 live model ids on the endpoint reject temperature ≠ 1 / top_p ≠ 0.95, and the official documents require omitting the fields entirely — the strip no longer depends on a model-id roster. The route twin (kimi_code_sampling_route) is pinned to the config-crate reference matcher on the shared host (5f5eca337); it is deliberately twin-only on api.kimi.ai until fix(tui): replay reasoning_content for kimi k3.1 #137 widens the shared matcher, which also means the K3 reasoning-dialect checks remain .com-only in the interim — an asserted, documented asymmetry, not a drift.
5. Digit-led tool names get a _ prefix to satisfy the documented name pattern — and the loop now closes on the response side (5f5eca337): engine dispatch falls back to the unprefixed registry name for the exact _<digit> wire shape (never stripping genuinely underscore-led names, and exact spellings still win when both exist), and the Moonshot message builder re-applies the prefix to replayed assistant tool_calls so the history matches the tools array. A round trip request → model call → dispatch now completes for such tools.
6. default passes verbatim on any node with any value per the specification's default section — the MFJS validator performs no type or placement check (pinned). Caveat: the shared provider-neutral passes that run before MFJS validation may still rewrite schema-shaped payloads inside a default value (e.g. a {"oneOf":[X]}-shaped default or a dangling required key in it); that is pre-existing engine behavior, not an MFJS-level type check.
7. Boolean/null const literals are translated through an internal enum channel and are stripped before anything ships: a typed node keeps its declared type with the pin constraint released; an untyped node fails closed with MissingType; object/array consts fail closed outright. Nothing boolean/null ever reaches the wire as an enum value.
8. Resource budgets align with the walle conf.go source defaults (500 / 3000 / 30 / 1000 / 75000), with exact-boundary, document-wide-accumulation, byte-budget, and structural-depth probes pinned (6d37a52cf). Scope caveat disclosed in-code: walle's MaxEnumItems is per-node while the engine total is document-wide.
9. Upstream e3e563204 (absent-items termination fix, released upstream in v0.10.2) is ported faithfully, with termination tests upstream did not have.
10. The shipped pptx schema is integrated cross-repository as a test fixture (crates/tui/tests/fixtures/pptx_make_pptx_schema.json), byte-exact to the companion change below.
11. The policy comments quote the governing documents verbatim, including the closing clause, and the 2026-11-04 live-probe record is demoted to observations-only; the three fixture comments that still spoke of "both route whitelists" and a "live-verified legal" pattern are corrected (c4b958992) — no route admits any probe-observed keyword.

Test evidence

Full workspace suite green on the fork CI for the series head; codewhale-tui counts 12,705 tests at 8e2882e3d4 (12,705 passed / 0 failed under nextest --all-features, plus 15 workspace-level skips) and gains 10 pinned tests in 5f5eca337 + 6d37a52cf (digit-led dispatch fallback, collision precedence, replay re-prefix, route-twin differential sync, empty-enum fail-closed, exact 75000 enum-text boundary, document-wide enum accumulation, byte budget, structural-depth guard, single-group constraint note). Local targeted sweeps on the final state: schema_sanitize 77/77, client:: 550/550, core::engine:: 715 green / 4 ignored, turn_loop 76/76.

Behavior changes

• Moonshot/Kimi requests stop carrying keywords the specification does not list (title, $comment, $id, min/max bounds stripped; everything else fail-closed and the tool dropped). Schemas relying on them lose those hints — or the whole tool — on this provider's wire only; other providers are untouched.
• type: ["string","null"] style type arrays and typeless/{} subschemas are rejected wholesale (the tool drops): the specification lists type as a single string, and the documents govern over walle's more lenient source. Previously the engine shipped the eight advisory keywords and type-checked default; both behaviors are gone.
• Root unions remain flattened (unchanged intent, now pinned to the quoted rule): branch-required groups are dropped and disclosed in the tool description note; conflicting cross-branch property definitions resolve to the first branch (undocumented approximation, now stated here); branch additionalProperties: false and non-object alternatives are dropped without a note.
• Fixed sampling on Kimi Code membership routes is omitted regardless of model id spelling, and now also on the overseas host (whose reasoning-dialect checks stay .com-only pending fix(tui): replay reasoning_content for kimi k3.1 #137).
• Digit-led tool names arrive at the provider with a _ prefix, and a call to such a tool now dispatches: the registry-side name is unchanged, dispatch resolves the prefixed spelling, and replayed assistant turns are re-prefixed. Typed boolean/null consts keep the tool with their pin constraint released; untyped ones fail closed.
• enum: [] and non-array enums fail closed (tool dropped) instead of being silently widened.

Companion change

Pinvou/pinvou-agent PR codewhale-hq#837 ("fix(mcp): land the T11 pptx schema and contract batch") ships the matching per-layout anyOf pptx schema in pinvou3-app/resources/mcp-servers/pptx/server.py; the fixture here was re-exported byte-exactly from that PR's head. codewhale-hq#837 must land for the new schema to ship anywhere — no main branch carries it yet — and it re-pins this repository's gitlink, so the two changes land as one coordinated batch.

Merge-conflict surface

• With fix(tui): replay reasoning_content for kimi k3.1 #137 (fix/kimi-k31-replay-spellings): this branch edits the top use crate::config::{...} hunk of crates/tui/src/client/chat.rs (removes is_kimi_code_membership_model and moonshot_base_url_is_exact_kimi_code imports); fix(tui): replay reasoning_content for kimi k3.1 #137 adds moonshot_base_url_is_exact_direct_platform there. The resolution is load-bearing: the union must drop is_kimi_code_membership_model (its only call site is deleted on this side; the workspace denies warnings, so keeping it fails CI) and must keep moonshot_base_url_is_exact_kimi_code (fix(tui): replay reasoning_content for kimi k3.1 #137's replay arm calls it). The semantic changes are independent.
• With fix(tui): pass xAI root unions of object branches through #144 (fix/xai-root-union-passthrough): both append tests at the same anchor in mod image_block_wire_tests; keep-both is safe (no identifier duplication).

Probe declaration

The 2026-11-04 live probes of https://api.kimi.com/coding/v1 recorded in this branch (keywords the endpoint answered 200 on) are observations only and never widen the wire: every shipped rule above is grounded in the published documents cited in this PR, and the probe record exists solely as motivation history and a tightening audit trail.

Disclosure

• Not every intermediate commit in the original 16-commit series compiles independently: f6b7eee4d, b91296cac, and ebef9348a reference the later-removed route-tier API (E0425/E0061). The per-commit gate counts quoted inside those three commit messages were not produced from those trees and should not be trusted; the follow-up commits (5f5eca337, 6d37a52cf, c4b958992) each compile and carry their own tests. A squash merge is unaffected; bisecting into the middle of the series can land on non-building states.
• The schema_sanitize:: + tools:: test pair fails auto_review_guardian_denial_refuses_the_call_with_a_receipt when run in one process in that order on the base commit itself (pre-existing shared-state flake, not introduced here); CI's ordering does not trigger it.

No-Issue: provider compatibility batch grounded in the published provider docs (fetched 2026-11-05); the repository has issues disabled.

[asto18089]

Review verdict: APPROVE — 0 blocker / 0 major / 18 minor / nits below

(Self-approval is blocked for the author account, so this is posted as a comment; the verdict above is the review outcome. Fresh multi-lane re-review at bed78918d, no rebase needed — the branch already contains the pinvou3-clean tip 11cba403d and is 0 commits behind.)

What was verified first-hand

• Spec conformance: the MFJS spec (EN + ZH) and the Kimi tool-use doc were fetched independently. The implemented 11-keyword whitelist ($ref $defs anyOf properties additionalProperties items type enum required default description) matches the spec's Field Definitions keyword-for-keyword and constraint-for-constraint. The nine advisory keywords are verifiably outside the supported set; walle model.go SupportedKeywords carries exactly 8 of them ($comment in neither set) — the in-code source-comparison note is verbatim accurate. All in-code spec quotes match the fetched documents verbatim.
• Budgets: all six constants + names verbatim-match walle conf.go DefaultValidatorConfig (500 / 3000 / 30 / 1000 / 75000 / 120000), every boundary direction (>) matches, the disclosed per-node vs document-wide caveat is accurate, and the engine is never looser than the server on any budget. The dropped composition walk (c24f8b0c5) was verified dead.
• Upstream port: e3e563204 exists upstream, is an ancestor of v0.10.2 (and not of v0.10.1), the base lacked it, the port is line-faithful, and "termination tests upstream did not have" is true.
• Fixture provenance: the pptx fixture is byte-identical to pinvou-agent PR codewhale-hq#837's head (221513a6) server.py TOOL_DEFS (reconstructed the programmatic schema build and compared byte-for-byte). The PR body's claim is true as scoped, and the unlanded-companion dependency is accurately disclosed.
• Smuggling: all 58 hunks classified; zero unrelated changes. client.rs diff is tests-only; config.rs is a doc comment. A base→head function-level comparison shows all shared-path sanitize functions byte-identical except three production functions plus the whitelist internals — non-Moonshot schema behavior is unchanged except where noted in M4.
• Tests/mutations: targeted sweeps green locally (schema_sanitize 77/77). 6/7 mutations killed (prefix removal, empty-enum widening, single-group note gate, dispatch fallback death, replay no-op, .ai host drop — all RED). The one survivor is a test-strength gap (M5), not a code defect.
• Disclosure accuracy: all three named non-compiling intermediates confirmed (E0425/E0061); no probe-admitted keyword survives anywhere in the final tree; the #137 merge-conflict description (drop is_kimi_code_membership_model, keep moonshot_base_url_is_exact_kimi_code) is exactly right against #137's head patch. The route/host matcher was exercised against an 18-URL matrix (platform hosts, lookalikes, ports, case, query, proxies) with no false positives on api.moonshot.*.

MINOR findings

1. Broken citation — commit 8e2882e3d cites parent commit 177dbce6, which does not exist (GitHub API 422 on both repos; invalid locally; not a force-push orphan). The message's substance is true; cite cf091b5a / f5986f96 or head 221513a6 instead.
2. False mechanism claim in the digit-led fix — chat.rs:1202-1204 and commit 5f5eca337 say "Assistant turns persist the registry spelling". Actually the session message is persisted (turn_loop.rs:2935) from tool_uses clones (:2877-2885) before plan_tool_calls (:3561) rewrites names (:3815-3818), so history keeps the wire spelling _3d_render. The loop still closes (shape-keyed, idempotent re-prefix + the dispatch fallback), but the replay test fixture's first half doesn't match live persistence and the comment should describe the real mechanism.
3. Dispatch fallback is engine-wide, not Moonshot-gated (turn_loop.rs:7528-7548) — a hallucinated _<digit> name now dispatches the unprefixed tool on any provider, in tension with the resolver's own doctrine (registry.rs:347-349, "a hallucinated name must fail, never dispatch"). Exact-spelling precedence holds and policy gates run on the resolved name, but the PR body frames this as Moonshot-only recovery. Either provider-gate the fallback or update the doctrine comment.
4. Cross-provider reach of the single-group note vs body claim — root_composition_constraint_note's gate is now !groups.is_empty() (schema_sanitize.rs:422), and sanitize_for_responses is shared with the Anthropic and OpenAI-Responses lanes (anthropic.rs:98, responses.rs:1228), so single-group notes now appear in those providers' model-facing descriptions too. Body bullet 1 says "other providers are untouched" — the behavior is fine, but the body should scope this.
5. Exact-75000 boundary test survives upward drift — empirically, mutating the constant to 75_001 stays GREEN: the "over" probe is +1000 (76000), not +1, so only the downward direction is pinned, while the test comment claims "a value edit that moves the budget silently fails here". The byte-budget test's kill zone is likewise ~129KB wide (~130k vs ~1.1k probes). Add ±1 probes for both.
6. User docs untouched — docs/PROVIDERS.md / config.example.toml still name only the .com membership host and mention sampling omission only for direct K3; nothing user-visible discloses (a) the .ai membership route or (b) that configured temperature/top_p is silently dropped for every model on membership routes.
7. K3 .ai asymmetry disclosed only at the twin — the interim .com-only scope of the K3 reasoning-dialect/replay/stream checks is stated at the sampling twin (chat.rs:956-959, 6978-6980) but not at the sites that actually degrade on .ai (chat.rs:183-186, 3633-3638, config.rs:9996), and nothing directly asserts is_exact_kimi_code_k3_route on .ai (the pin is transitive through the twin test).
8. Both-spellings registry → duplicate wire names — with both 1x and _1x registered, the tools array ships two entries named _1x (no dedupe, untested); provider behavior for duplicates is undefined. Dispatch resolves correctly, but the wire shape this fix exists to make valid is left undefined for this corner.
9. Pin gaps — ConflictingLiteralConstraint (enum+const disagreement → drop) has zero test coverage; default at root/$defs/items is code-verified but unpinned (the test covers properties positions only); the traversal asymmetry around default (provider-neutral passes rewrite inside default payloads, MFJS-layer passes don't) is body-disclosed but undocumented in code.
10. Unbounded constraint note — group count and note length are uncapped (schema_sanitize.rs:406-430, chat.rs:1134-1138); a ≤500-branch union can append tens of KB to every request's tool description. Consider a cap + truncation tail.
11. Flatten disclosure gaps — additionalProperties: false closure loosening and non-object-alternative narrowing are disclosed only in the PR body (no code comment, no note clause). Preserving additionalProperties: false when all object branches declare it would be provably lossless.
12. Fixture pins an open companion without a byte-level sync mechanism — codewhale-hq#837's required CI is currently failing, so its head will likely move; structural drift fails loudly here, but wording drift (descriptions, enum values) would be silent. A provenance comment with the SHA, or a fingerprint assert, would close it.
13. $ref walkers lack a step budget — walle bounds the equivalent walk (maxRefWalkSteps = 500000); the engine's terminator/depth walkers re-walk shared $ref targets per path and clone the visiting set per branch/required, so a diamond-ref document under the 64-cap can cost exponentially many walks. Pre-existing and adversarial-only, but it is the one walle guard missing in the area this PR aligns.
14. Clone + walks before the size check — the whole-schema deep clone and all walks execute before the 120KB budget check (schema_sanitize.rs:1313 vs 1791); an oversized schema pays full traversal before being dropped. A cheap byte pre-check on parameters would fail fast.
15. Sibling name-pattern class uncovered — registry names whose escaped wire form is --led (non-ASCII digits ٣/１ → -x00663-…, punctuation-led names) still violate ^[a-zA-Z_]… and cannot round-trip on Moonshot; pre-existing via to_api_tool_name, but it's the same documented-pattern rationale this PR applies to digit-led names.
16. 128→129 length bound — a 128-char digit-led name becomes 129 after the _ prefix, outside the documented {0,127}; no rejection or handling. Pre-existing exposure (a 128-char digit-led name was already invalid by its leading digit), noted for completeness.
17. int+float mixed enum over-strip — enum: [1, 1.5] on a "number" node is stripped (Integer ≠ Number kinds) although the validator — and walle — would accept it; conservative direction, undisclosed.
18. Stale/mismatched comments — the fixture header comment (schema_sanitize.rs:4115) still says "the MFJS branch budget used to the last slot", contradicting the honest sentinel comment at :4183-4186 after the budget went 10→500; note-attachment separators diverge across lanes (space vs \n\n); tool_choice re-implements the digit-led predicate inline instead of reusing prefix_digit_led_function_name.

NITs (not tracked)

default not admitted as a $ref sibling (policy-consistency question; fail-closed is the safe direction); empty anyOf: [] reports ResourceLimitExceeded instead of a precise variant; enum-strip/bounds-strip degradations carry no description note (notes reserved for required-contract relaxations — consistent policy); root enum/not removal is silent in all lanes (pre-existing); KimiParameterSchemaError variant fold changes Moonshot warn-log wording; module note says pattern is in walle's FutureKeywords when it is also in StringAllowedKeywords; sanitize_for_kimi silently drops an unsatisfiable parent-type intersection; path_suffix config can bypass the route twin (shared blind spot with the config matcher); ASCII-only boundary probes can't detect a bytes-vs-chars switch; byte-measure diverges from walle at the margins (Go HTML-escaping/float formatting — safe direction); termination tests guard end-to-end behavior but would stay green if only the ported early-return reverted (defense-in-depth line unpinned directly); unreachable-path comment at :2236-2238; strip runs before validation but {"$ref":…, "default":…} fails closed while {"$ref":…, "title":…} survives (title stripped upstream).

Merge notes

• The disclosed #137 conflict surface is verified correct. One addition: besides the declared mod image_block_wire_tests anchor conflict, #144 also textually overlaps #143's chat.rs hunk at old lines ~1178-1192 (Moonshot comment rewrite vs #144's xAI comment rewrite) — keep-both resolution, but the section presents itself as exhaustive.
• Companion codewhale-hq#837 must land for the fixture's schema to ship anywhere, and it re-pins this repo's gitlink — the coordinated-batch framing is accurate.

Verdict stands: the PR does what it claims, grounded in the published documents, with unusually strong disclosure. The minors above are follow-up material; none blocks on correctness.

[JensenChen28]

Requested changes for the reviewed head bed78918d2f2717c7a5cb03120edbb9f93720d35.

I found two behavior issues: the bool/null const path widens executable tool schemas, and the digit-prefix recovery is applied globally rather than only to Moonshot wire names. Both have inline details below.

Verification: git diff --check passed for both PR diffs. GitHub checks on this head are green (Rust analysis, fork CI, CodeQL, type check, DCO, Gitleaks); the sync job is skipped. I did not run local Cargo tests in this cycle.

[asto18089]

Both P2s fixed (new head d31e59b8f):

• Bool/null const widening: is_mfjs_scalar_const now accepts only the enum literals MFJS documents (string/number). A typed or untyped bool/null const fails closed with UnsupportedConstLiteral — the tool drops with a projection warning instead of shipping the bare declared type that silently widens the author's pin (const: true becoming "any boolean" nothing re-validates). The widening-asserting test is replaced by kimi_parameters_fails_closed_on_bool_and_null_const. Disclosed sibling, unchanged here: a declared bool/null enum still strips to its bare type through the pre-existing unsupported-enum-value path — same widening family, outside this PR's const-translation scope; happy to take that up as a follow-up if reviewers want it in this PR.
• _<digit> alias recovery: now gated by the installed route's provider receipt (matches!(self.api_provider, ApiProvider::Moonshot) — the same identity the request-side prefix uses). From any other provider the spelling is a hallucinated name and fails dispatch per the registry invariant; regression non_moonshot_wire_never_recovers_the_digit_led_spelling pins both the refusal and the exact-lookup path for bare digit-led names.

Full tui suite green locally (12432 passed / 0 failed).

One merge-order note reviewers should know: the hand-copied kimi_code_sampling_route twin introduced by this PR mirrors the config matcher's trailing-slash semantics via the sync pin at chat.rs; the separate in-flight PR #145 changes those semantics (all trailing slashes insignificant). Whichever of the two lands second needs the pin's documented one-line flip (drop coding/v1// from the agree-list is NOT sufficient — the twin itself takes trim_end_matches). Left per that pin's own procedure to avoid pinning a mismatch inside either individual PR.

Note on CI: the failing checks (Sync-to-CNB credentials, CodeQL Analyze) fail repo-wide since 2026-11-11 — unrelated to this PR's content.

[JensenChen28]

One repository-policy blocker remains after the two behavior fixes. The new route matcher is a documented twin of the existing config-crate matcher; please centralize the URL matching/normalization so the two consumers cannot drift, while retaining the intentionally different Kimi Code host set at the caller if needed. I found no additional spec-axis blocker. CI checks are not providing a code signal: GitHub reports the account is locked due to a billing issue and jobs failed before start.

[JensenChen28]

The two reported issues are fixed in this head: bool/null const values now fail closed, and _digit recovery only runs for Moonshot responses. The added tests cover both contracts. I found no new material issue in the reviewed change. git diff --check passed; I did not run local Cargo tests (cargo is unavailable). GitHub workflow runs currently show failures, but their logs were not available through the review connector.

[JensenChen28]

Requesting one focused follow-up on the reviewed head. The earlier approval did not account for the newly surfaced unresolved route-matcher thread; after rechecking this head, I agree it is a material repository-policy concern. kimi_code_sampling_route reimplements scheme/authority/path parsing and trailing-slash normalization already in codewhale_config::provider::is_exact_https_route, and its own comment says those semantics must stay synchronized. The current differential test covers the shared .com host, but leaves two independently maintained implementations and makes future semantic changes (including the distinct .ai host policy) easy to apply inconsistently. Please move the reusable URL-matching primitive to a shared location/API and keep the host allowlist at the caller, or otherwise demonstrate a repository-approved shared approach. git diff --check passed; I did not run Cargo tests in this cycle, and the reported failing workflow logs were unavailable.

[JensenChen28]

Reviewed head c154a4d0364d7af3d7fe859954feade644041c28. The shared exact-route primitive now removes the duplicated matcher while keeping the distinct Kimi Code host policy at the caller; the previously raised route-drift concern is resolved. I found no remaining substantive blocker, and all review threads are resolved. git diff --check 6f7a3a748f408568468ace45aa5cdbb34f882d73...HEAD passed; I did not run Rust tests because cargo is unavailable in this environment. Non-blocking: please refresh PR description §4 and its commit/head count, which still describes the old twin-only matcher and an older series state.

[zhuowp]

Reviewed c154a4d0364d7af3d7fe859954feade644041c28 against actual target 6f7a3a748f408568468ace45aa5cdbb34f882d73. The cumulative changes and affected producer → request → dispatch → replay chains have been reviewed. Two correctness issues remain:

1. [P2] Allocate distinct Moonshot wire identities before advertising tools. In crates/tui/src/client/chat.rs:1172–1185, 3d_render and _3d_render both become _3d_render. Both are valid, distinct plugin/registry keys and can coexist in the active catalog. Both response decoders preserve that spelling, and resolve_tool_definition (turn_loop.rs:7492–7494) selects the genuine underscore tool before the recovery fallback. A call selected using the digit tool's schema therefore reaches the other handler, and forced tool choice has the same ambiguity. The collision test checks resolver precedence without projecting the two-tool catalog, so it does not establish a safe round trip. Use a reversible collision-free catalog mapping consistently for advertisement, forced choice, response resolution and history, or explicitly handle the collision before sending. Existing permission gates still run; this finding does not claim an authorization bypass.

2. [P2] Bound reference-validation work when widening schema admission. The larger limits at crates/tui/src/tools/schema_sanitize.rs:1836–1838 admit inputs for which validate_mfjs_expanded_depth (2262–2337) repeatedly recomputes already visited subproblems. Its visiting set guards the current recursion path, while the physical-document limits and depth checks do not bound total work. Validation runs synchronously inside sanitize_for_kimi_parameters during request preparation, before per-tool degradation can complete. Source analysis and bounded isolated probes confirm a substantial preparation-cost regression for newly admitted finite schemas; this is more than a theoretical concern about the old walker. Add a context-correct memoized/bounded graph traversal or an explicit work limit while retaining the intended supported forms. Detailed resource-exhaustion constructions are deliberately omitted from this public review.

Two secondary issues are confirmed but would not independently block approval: newly accepted schema-shaped default payloads can still be rewritten by the earlier shared sanitize_for_responses/registry passes (schema_sanitize.rs:1349,2077); and the single-group note at 418–425 can incorrectly require a field when another anyOf alternative accepts an empty object. These affect model guidance rather than enforced arguments or durable data. Preserve literal-data positions through every preprocessing pass and describe optional alternatives accurately. The PR's default caveat discloses the limitation, but the new verbatim-preservation test does not cover it.

The previous bool/null-const widening, non-Moonshot fallback and duplicated route-matcher findings are fixed in this head. The PPTX fixture matches the current companion source, whose actual gitlink still points to the old engine; no future repin was assumed. Validation here combines static production-chain review with isolated fixed-source helper checks, including positive/negative fixture controls. It does not claim a native application build, live provider execution or release qualification. CI issues are not the basis for this decision.

[JensenChen28]

Two material P2 issues remain on this head; I independently confirmed both affected paths. git diff --check 6f7a3a748f408568468ace45aa5cdbb34f882d73...c154a4d0364d7af3d7fe859954feade644041c28 passed. Rust tests were not run because cargo is unavailable in this environment.

[JensenChen28]

Standards

No hard documented-standard violation or blocker in this delta. The collision handling is limited to the Moonshot request path, rejects forcing the dropped registry name, and has colocated wire/forced-choice regressions. The reference walk now memoizes by node and depth and has a total-state cap; its incompatible-resource path remains fail-closed.

Spec

The prior collision and expanded-reference work findings are addressed: the digit-led entry is omitted when minting its wire spelling would alias an advertised underscore tool, while the underscore entry remains forceable; shared $ref work is memoized and bounded. No new blocking source issue found.

Non-blocking: the PR description still says the route matcher is a twin-only .ai exception and gives the old 20-commit/bed78918d state. Please refresh it to match the already-shared matcher and current series.

CI: current runs fail within 1–3 seconds with empty job step lists, so the checks did not execute. No local Rust tests were run for this review.

[JensenChen28]

Reviewed the current head 6ba452f245c28b11cff04c350a8a93c7a70e5f0c against the PR's documented MFJS/tool-use contracts and the current target branch. The two earlier findings have fixes, but three boundary cases remain that can suppress a usable tool or leave the request-preparation path unbounded. Details are inline. git diff --check origin/pinvou3-clean...HEAD passed. I could not run Rust tests because cargo/rustc are unavailable in this environment.

[JensenChen28]

The latest delta addresses the three prior P2 findings: Moonshot schema filtering now precedes collision detection and forced-choice checks use the surviving registry-to-wire mapping; the MFJS termination prepass now shares the total-work cap; and digit-led names are dropped when prefixing would exceed the documented 128-character limit. The added regression cases cover these boundaries. No additional blocker found in this delta. CI checks are failing immediately in this run, so this approval is based on source review.

[JensenChen28]

Reviewed the new head 681a171b1bc33789e5f8ba97072506eba3362788. The three prior findings are fixed in this delta, but I found one new P2 forced-choice regression: an omitted underscore registry tool can be mistaken for a surviving digit-led tool's minted wire alias. Inline details below. git diff --check passed; I could not run Rust tests because cargo/rustc are unavailable.

[JensenChen28]

The latest delta fixes the forced-choice alias regression: the offered set is now captured after schema sanitization but before digit-name minting, while forced digit-led names are still validated against the surviving registry-to-wire mapping. The regression rejects forcing schema-dropped _3d_render while preserving the valid 3d_render path. No new source-review blocker found. The latest CI checks fail within seconds across jobs; no tests were run locally.