Repository navigation
Conversation
The Kimi Code membership endpoint answers any sampling value other than the fixed ones with a 400 on every model it serves: a 2026-11-04 live probe (GET /models plus per-model requests) showed all six served ids - k3, k3-256k, k3.1-pro, k3.1-pro-highspeed, kimi-for-coding, kimi-for-coding-highspeed - returning "invalid temperature: only 1 is allowed for this model" / "invalid top_p: only 0.95 is allowed for this model". The strip was gated on the membership roster, so the two k3.1-pro spellings the roster does not list sent user-configured temperature/top_p straight into that 400. This is the roster's third lag behind the endpoint (k3-256k and kimi-for-coding-highspeed before k3.1-pro), so the gate stops consulting it: apply_kimi_code_fixed_sampling now strips temperature/top_p from every request on the exact https://api.kimi.com/coding/v1 route regardless of model id, and its unused model parameter is gone. The direct Moonshot platform strip (api.moonshot.ai + kimi-k3, model-scoped) and the custom-gateway boundary are untouched - the membership contract never leaks to a neighboring base_url, provider identity, or platform endpoint. The roster doc now states that sampling stripping is deliberately not one of its consumers, so nobody re-couples them. Red-green evidence: kimi_code_fixed_sampling_covers_every_model_id_on_ the_route (six probed ids plus an unserved id, through the wire builder), the extended kimi_code_compaction_shape_omits_sampling_parameters_on_wire (roster ids plus both k3.1-pro spellings end to end), and the rewritten kimi_code_fixed_sampling_does_not_leak_to_neighbor_routes (platform, gateway, and non-Moonshot provider boundaries) fail against a temporary reintroduction of the roster gate and pass after its removal; direct_moonshot_k3_uses_modern_token_field_and_fixed_sampling_only_on_ exact_route pins the unchanged platform strip. client:: tools:: gate 2639 green. Signed-off-by: asto <asto18089@126.com>
The Moonshot compatibility passes still carried narratives written when the behavior was different, with no marker distinguishing history from the present: drop_dependent_keywords described the long-gone whole-request build failure in the present tense, the kimi tests section header claimed composition stays available only in nested anyOf positions, and the candidate-pass note said a rejected shape would be 'failing the whole turn'. Keep every historical account - the incident narratives are the record of why each guard exists - and add the current-state sentence to each: the request builder now degrades per tool (drop + projection warning), the keyword whitelist is route-aware (KimiParameterRoute), and a root union of explicit object branches ships since the walle specification listed anyOf as an applicator and the 2026-11-04 live probe of https://api.kimi.com/coding/v1 confirmed it (200, per-branch required honored). No behavior change; 186 schema_sanitize/moonshot/kimi-matching tests green. Signed-off-by: asto <asto18089@126.com>
The parent repository's 2026-11-04 batch reshaped the shipped make_pptx slides schema (pinvou3-app/resources/mcp-servers/pptx/server.py, TOOL_DEFS) into a per-layout anyOf so constrained tool-call decoding fills per-branch required fields, and this fork's Moonshot layer grew the validator that must accept it - but the two sides had never been proven against each other end to end. Export the tool definition verbatim from the parent tree into a JSON fixture (tests/fixtures/pptx_make_pptx_schema.json, extracted from the parsed TOOL_DEFS with a read-only ast script: 10 explicit object branches, 47 properties, single-value layout enums, per-branch required groups, nested agenda/kpi item unions, typed chart series) and pin the seam: shipped_pptx_slides_schema_passes_moonshot_validation_on_both_routes runs sanitize_for_kimi_parameters plus validate_mfjs_parameters on both KimiParameterRoute values and asserts the union survives - slides.items stays an anyOf of 10 branches (never flattened into a merged object), every branch keeps its layout enum and required group, the nested agenda/kpi unions keep both branches, and the plain object root with required ["slides"] is untouched with no degradation note. The fixture budgets sit exactly at the MFJS limits the schema author cites (10/10 branches, 47/100 properties, depth 3/5, 13 enum values), so either side's drift turns the test red: a schema regression past a budget in the parent tree, or a renewed flattening of nested unions in this sanitizer. Green against the current head on the first run - no seam between the two batches. schema_sanitize:: tests 74 green; cargo fmt -p codewhale-tui --check clean. Signed-off-by: asto <asto18089@126.com>
Kimi Code's official documentation presents two regional membership endpoints for the same product (codex.html, "Kimi Code API address"): China https://api.kimi.com/coding/v1 and overseas https://api.kimi.ai/coding/v1 (re-verified 2026-11-05). The fixed sampling strip reached the wire through the China-only predicate, so an overseas user whose configured base_url points at the .ai host kept generic temperature/top_p on the wire and hit the exact 400 class this strip exists to prevent ("invalid temperature: only 1 is allowed for this model"). The strip now matches through kimi_code_sampling_route, a route-scoped twin of codewhale_config::provider::is_exact_https_route spanning both hosts with identical semantics: scheme and authority ASCII case-insensitive, path exact, a single trailing slash insignificant, neighboring paths and lookalike hosts never match. The config-crate helper is private and widening is_exact_kimi_code_route belongs to in-flight PR #137, so the twin lives in chat.rs and nothing under crates/config changes. The KimiParameterRoute selection deliberately stays on the China-only predicate: the relaxed MFJS keyword tier is probe-backed for .com only (2026-11-04, 6/6 models on the China endpoint), so .ai keeps the conservative platform whitelist until an independent probe or PR #137 folds it in. overseas_kimi_ai_endpoint_keeps_the_conservative_parameter_ whitelist pins that boundary at the wire level - a pattern-carrying tool still degrades per tool on .ai while sampling is stripped - and sanitize_moonshot_chat_tools' doc no longer claims the two predicates are the same. The extrapolation's failure mode is benign: not sending temperature/top_p is the officially documented configuration for this endpoint. Also correct the Platform route documentation, which read as "byte-identical to the f1eb10e baseline": relative to that baseline the platform route carries two intentional behavior changes - root level object unions pass through (anyOf/oneOf with all-object branches no longer flattened; backed by the walle MFJS specification listing anyOf as a first-class applicator plus the 2026-11-04 probe), and an array with minItems >= 1 and items absent counts as terminating instead of failing the tool (ported from upstream codewhale-hq e3e5632, the codewhale-hq#7015 review follow-up). The keyword whitelist itself, nested allOf rejection on the platform route, and per-tool degradation stay baseline-consistent. Docs only; no behavior change in this part. Tests (the two new ones red before the predicate change, green after): - kimi_code_fixed_sampling_covers_the_overseas_api_kimi_ai_endpoint (.ai exact route, trailing slash, uppercase host) - overseas_kimi_ai_endpoint_keeps_the_conservative_parameter_whitelist - kimi_code_fixed_sampling_does_not_leak_to_neighbor_routes extended with the api.kimi.ai/coding/v2 and api.kimi.com/coding/ neighbors; existing neighbor non-leak and every-model-id coverage stay green. Gates: cargo test --locked -p codewhale-tui --lib -- client:: tools:: 2643 green (the forkguard_model_operation_auxiliary_retry_keys full-concurrency flake stays green single-run; the full-lib run still aborts with a SIGABRT stack overflow reproduced identically on clean 13f95e961, so it is pre-existing and unrelated); config:: 508, route_billing 46, kimi/moonshot filters 159 green. cargo fmt --all -- --check clean. Workspace clippy with the CI lint parameters reports only the pre-existing clippy::collapsible_if at crates/tui/src/core/engine/turn_loop.rs:5499 (introduced by 6ba52af, verified present on clean head) - nothing in the touched files. Signed-off-by: asto <asto18089@126.com>
The 2026-06 Kimi incident shape - a parameters root anyOf whose branches carry their own required groups - was flattened into one merged object by sanitize_for_responses, erasing every branch's required set. k3's constrained decoding then only ever emitted the single merged group on long contexts (the real pptx-tool case: the wire schema was left with one required set where the schema author had written two). Moonshot's MFJS has since made this shape legal. The walle specification (https://github.com/MoonshotAI/walle/blob/main/docs/mfjs-spec.zh.md) lists anyOf as a first-class applicator - type must live inside the branches and every branch must be an object schema - and its response_format guide states k3/k3.1-pro reliably support structured output including anyOf. A live probe of https://api.kimi.com/coding/v1 on 2026-11-04 returned 200 for a root anyOf of object branches with per-branch required honored, and accepted the parent-side type of the 2026-06 incident form without error. Narrow the flattening accordingly. sanitize_for_kimi_parameters now passes a root union through when a single anyOf/oneOf carries only explicit type:"object" branches beside legal union siblings (type, title, description, root-only $defs/$id), promoting a root oneOf to the documented anyOf; normalize_kimi_compatibility's type-pushdown then moves a parent-side type into the branches so the sent shape matches the specification. validate_mfjs_parameters accepts the union root while its union-sibling check still rejects a type beside the keyword. Anything else - a union with a non-object branch (apply_patch's bare required-groups among them), both union keywords at once, root allOf/enum/not, or an unrepresented keyword beside the union - keeps the flatten and its constraint note, so no unverified shape ships. Red-green evidence: kimi_parameters_passes_root_anyof_of_object_branches_ through, kimi_parameters_promotes_root_oneof_of_object_branches_to_anyof, kimi_parameters_pushes_root_type_into_object_union_branches, mfjs_ validator_accepts_union_root_but_rejects_union_siblings, and kimi_chat_ keeps_root_union_of_object_branches fail against the old implementation (root union folded, branches erased) and pass after the narrowing; kimi_parameters_add_type_to_anyof_root (null branch), kimi_parameters_add_ type_to_allof_root, kimi_parameters_still_flattens_root_union_with_non_ object_branch, kimi_parameters_still_flattens_root_union_with_extra_root_ keywords, and kimi_parameters_flattens_actual_apply_patch_root_and_returns_ constraint_note pin the preserved flatten. 72 schema_sanitize-matching tests green; client:: tools:: gate 2634 green. Signed-off-by: asto <asto18089@126.com>
A 2026-11-04 live probe of https://api.kimi.com/coding/v1 verified end to end (200, and the model honored the constraints) that the endpoint accepts pattern, format, $schema, prefixItems, exclusiveMinimum, and exclusiveMaximum, plus nested allOf - keywords the walle MFJS specification does not list and that therefore risk a whole-request 400 elsewhere. The Moonshot tool-schema gate previously applied one conservative whitelist to every Moonshot route, so an MCP tool carrying any of these keywords was dropped from every Kimi Code request with only a projection warning. Make the MFJS gate route-aware. sanitize_for_kimi_parameters and validate_mfjs_parameters take a KimiParameterRoute: the exact coding endpoint (matched with the same predicate as its fixed-sampling shaping, so a user-configured base_url pointing there gets the same treatment) adds the live-verified keywords to the whitelist with value checks (strings for pattern/format/$schema root-only, array-typed prefixItems schemas, numeric exclusive bounds folded into the range check), passes nested allOf through (two-branch shapes; single-branch allOf already collapses semantics-preserving, and the provider-neutral injection's empty properties map on allOf nodes is stripped as the same no-op), and keeps every core fail-closed rule (boolean/tuple items, empty branches, inverted bounds). Platform endpoints keep the conservative whitelist unchanged - no live evidence there, and the 400-risk asymmetry stands - so per-tool degradation and the ToolProjectionWarning stay as the final safety net on the coding route too. Also port codewhale-hq e3e5632 (the codewhale-hq#7015 review follow-up): MFJS allows items to be absent and an unconstrained array can always be satisfied, so a required array with minItems >= 1 and absent items no longer fails mfjs_schema_can_terminate with InvalidSchemaNode - the residual drop after the empty-items repair. Applied on both routes since it is upstream behavior alignment, not a whitelist relaxation. The incompatible-tool fixtures in the three Moonshot request-boundary tests move from pattern (now legal on the coding route) to patternProperties (outside both whitelists) so they keep pinning the per-tool drop, the one-projection-warning stream event, and the named-tool_choice guard. Red-green evidence: kimi_code_route_ships_probe_verified_keywords, kimi_code_route_keeps_core_mfjs_rules_strict, kimi_parameters_min_items_ without_absent_items_terminates_upstream, kimi_code_route_keeps_probe_ verified_keywords_on_wire fail against the disabled relaxed layer (and the old terminator) and pass after; platform_route_keeps_the_conservative_ whitelist and moonshot_platform_route_keeps_the_conservative_whitelist pin unchanged platform behavior on both the validator and the wire; moonshot_drops_only_incompatible_tool, moonshot_rejects_named_choice_for_ omitted_tool, and moonshot_stream_emits_one_projection_warning keep the degradation path green. client:: tools:: gate 2639 green. Signed-off-by: asto <asto18089@126.com>
Reinstate the strict wire contract for Moonshot function parameters: a root oneOf/anyOf/allOf is flattened into a single type:object schema, as before 96fa83929. The Tool Use documentation requires "The root of `parameters` must be an `object`" (the MFJS specification lists anyOf as an applicator without authorizing it at a parameters root), and the new ruling is that the documents govern the wire shape — a 2026-11-04 live probe observing 200 for a root union is an observation, and per policy never a shipping basis against the documents. When the documents conflict, the strict face wins. Everything else from the 2026-11-04 batch is untouched: the nested oneOf-to-anyOf conversion, the type pushdown into anyOf branches, const-to-enum, nullable-to-anyOf, the empty-items repair, and the items-absence termination port. The union tests return to the baseline expectations (kimi_parameters_add_type_to_oneof_root pins the flatten; kimi_chat_flattens_root_union_of_object_branches pins it end to end on the Kimi Code wire), replacing the four union-passthrough tests. Red-green: kimi_parameters_add_type_to_oneof_root and kimi_chat_flattens_root_union_of_object_branches failed against the union-passthrough implementation (root union kept on the wire) and pass after the revert; 71 schema_sanitize tests, 156 moonshot/kimi-matching tests, and the 2684-test client:: tools:: gate green (one unrelated pre-existing flake, tools::file::read_file_byte_truncation_, green standalone twice). Signed-off-by: asto <asto18089@126.com>
Replace the probe-driven route tiers with one pipeline governed by the MFJS specification itself (https://github.com/MoonshotAI/walle/blob/main/docs/mfjs-spec.zh.md, fetched 2026-11-05). The governing ruling: the wire must follow the published documents strictly — a live probe may only ever tighten it, never widen it, and the 2026-11-04 observation that the coding endpoint accepted pattern/format/$schema/prefixItems/exclusive bounds/nested allOf is an observation only. The specification's closing clause leaves every unlisted keyword unsupported, so the KimiParameterRoute split (Kimi Code tier admitting probe-observed keywords, platform tier conservative) is deleted: every Moonshot route — api.moonshot.cn/ai, api.kimi.com and api.kimi.ai coding endpoints — serves the same document and gets the same wire contract. Two doc-conformance passes are added ahead of validation, each removing only what the document cannot express and whose removal never drops the tool: - strip_mfjs_advisory_keywords removes the annotation keywords (title, $comment, $id) and the unlisted bound keywords (minLength, maxLength, minimum, maximum, minItems, maxItems) — semantics-lossless or soft-constraint-losing simplifications. - strip_mfjs_unsupported_enum_values removes an enum whose values are not homogeneous float/int/str scalars ("枚举值我们只支持基础类型中的 float, int 和 str……不支持混合类型"); an enum-only node has no type to fall back to and still fails closed. The validator whitelist shrinks to the specification's supported set exactly: $ref (internal, root-ref pre-inlined), root-only $defs, anyOf, properties, additionalProperties, items, type, enum, required, default, description. The walle source (SupportedKeywords, ValidateLevelDefault==ultra) additionally carries min-max/title/$id, but the published specification does not — on the conflicting documents the specification text governs. Everything unlisted and unstrippable (pattern, format, $schema, prefixItems, exclusive bounds, nested allOf) keeps the baseline whole-tool drop with its ToolProjectionWarning. Kept: nested oneOf-to-anyOf, type pushdown, const-to-enum, nullable-to-anyOf, the empty-items repair, the items-absence termination port, the route-scoped fixed-sampling strip, the xAI union passthrough, and the MiniMax gateway gating. Malformed or inverted bounds no longer drop anything: the strip removes the keyword before any value is validated (kimi_parameters_strips_bounds_instead_of_rejecting_them). Red-green: probe_observed_keywords_fail_closed_on_every_route, advisory_keywords_are_stripped_not_shipped, unsupported_enum_values_are_stripped_and_tool_survives, probe_observed_keywords_never_reach_the_wire, advisory_keywords_are_stripped_from_the_wire, and overseas_kimi_ai_endpoint_keeps_the_document_parameter_whitelist failed against the route-tier implementation (probe keywords shipped on the Kimi Code tier, advisory keywords shipped everywhere) and pass after the shrink; shipped_pptx_slides_schema_passes_moonshot_validation_on_the_ document_set now pins the single pipeline plus a wire-keyword matrix (every keyword on the normalized pptx document must sit inside the documented set, layout/title names being property names). 71 schema_sanitize tests, 156 kimi/moonshot/sampling tests, and the 2684-test client:: tools:: gate green (the documented pre-existing forkguard_model_operation_auxiliary_retry_keys flake green standalone). Signed-off-by: asto <asto18089@126.com>
Record the 2026-11-05 ruling in the module documentation: the wire follows the published MFJS specification strictly, live probes are observations that may only ever tighten the contract, and when the walle source and its published specification disagree the specification text governs (with the concrete source-comparison note: SupportedKeywords carries title/$id/min-max and ValidateLevelDefault==ultra, none of which the document lists). One pipeline serves every Moonshot route because all of them document the same specification. Rewrite the fixed-sampling narrative to the same standard: the shipping basis is the documented "omit them from requests" contract, with the 2026-11-04 6/6-model probe kept as an observation record, and the overseas host joining on the document rather than on probe extrapolation. No behavior change; 71 schema_sanitize tests green, clippy and fmt clean. Signed-off-by: asto <asto18089@126.com>
The depth walker still visited allOf/prefixItems branch schemas, a leftover from the route-tier whitelists. Those keywords sit outside the documented set and fail the keyword whitelist before any depth walk can see them, so the visits were dead code. The walker returns to the baseline shape (properties, items, additionalProperties, anyOf). Signed-off-by: asto <asto18089@126.com>
The Moonshot MFJS specification's default clause is explicit: "default 字段为目标输出提供一个默认值……这儿不做类型检查，需要调用者自行保证 其值类型内容和所在位置期望的内容相匹配" — any value, any node, and the caller owns the value's correctness (https://github.com/MoonshotAI/walle/blob/main/docs/mfjs-spec.zh.md, fetched 2026-11-05). The engine was stricter than the document on two counts: mfjs_keyword_allowed_for_type admitted default only on scalar nodes, and validate_mfjs_default failed the whole tool closed when a default's value did not match the node's declared type. Both over-tightenings are removed: default now sits beside every concrete type (object and array included) and its value ships unchecked, so a third-party tool declaring {"type":"object","default":{}} keeps shipping instead of being dropped wholesale. The type-absence rule is untouched — a node carrying only a default still fails MissingType. Signed-off-by: asto <asto18089@126.com>
…faults The five engine-local budgets (anyOf items 10, total properties 100, object depth 5, enum items 500, enum string length 7500) had no annotated basis and sat an unexplained 10x under the walle validator's own defaults, so a third-party schema the server's validator would accept was dropped from every Moonshot turn. They now mirror conf.go DefaultValidatorConfig (source comparison, fetched 2026-11-05, declared in the constant block as a source note rather than a document clause): 500 / 3000 / 30 / 1000 / 75000. MaxSchemaSize already matched; the enum-length check threshold stays engine-local (the same budget, armed earlier); MFJS_MAX_STRUCTURAL_DEPTH stays as the engine's own recursion guard. Limit tests now pin both faces of each boundary — the aligned value ships, one past it fails — and the pptx sentinel moves from the old branch-count==budget coincidence to branch count far below the aligned budget with the full pipeline still green. Signed-off-by: asto <asto18089@126.com>
A bool/null const failed the whole Moonshot tool closed while a declared bool enum was stripped with the tool surviving — two shapes of the same information taking opposite degradation paths. The const handling now admits every scalar: bool/null consts translate through the existing const→enum transform to enum:[v], and the existing unsupported-enum-value strip removes what MFJS does not support, so the node lives on through its declared type exactly like a declared bool enum does. Non-scalar consts (object/array) keep failing closed; type inference stays limited to the string/number kinds the strip never has to invent, so an untyped bool/null const fails MissingType once its translated enum is stripped, matching the module's existing enum-only-node doctrine. Signed-off-by: asto <asto18089@126.com>
…plemented Pin the degradation ruling next to the code it governs: lossless annotations and soft numeric bounds strip so the wire keeps the document's supported set, while form keywords with no document support that materially steer generation (pattern, format, prefixItems, ...) keep the baseline whole-tool removal — better a missing tool than an unguaranteed shape. The walle source's FutureKeywords bucket for pattern and friends is recorded as a corroborating source-comparison note only. Also annotate the 2^53-1 integer bound as a JSON interoperation ceiling (not a document clause) and collapse the duplicated probe-schema block in the every-route fail-closed test into one run with one assertion message — one pipeline serves every route, so the second identical run carried no information. Signed-off-by: asto <asto18089@126.com>
…attern
Kimi's function-call name pattern is ^[a-zA-Z_][a-zA-Z0-9-_]{0,127}$
(platform.kimi.ai tool-use documents, fetched 2026-11-05) — a leading
digit sits outside the supported set, and to_api_tool_name only escapes
characters the pattern cannot express at any position, so a tool named
3d_render went onto the wire outside the documented pattern. The
Moonshot path now gives such API tool names a '_' prefix; forced
tool_choice carries the same wire spelling so the request stays
self-consistent and the dropped-tool guard compares like against like.
Every other provider keeps the unmodified name — their documents carry
no leading-character ban.
Signed-off-by: asto <asto18089@126.com>
The shipped fixture had drifted twice from the parent repo's pptx server.py: the slides description still carried the pre-177dbce6 '只给 title 会得到空白页' wording, and the agenda/end required groups predated the 2026-11-05 required audit (agenda now [layout, items], end [layout] — their empty titles render the 「目录」/「谢谢」 business fallbacks, so title is optional; the other eight branches are unchanged, cover/section/image layout+title and the six body layouts layout+title+body group). The fixture is regenerated byte-exactly from the current parent server.py by AST-evaluating its TOOL_DEFS literal — validated by regenerating from the pre-change revision and diffing against the stale fixture, where the description line was the only delta. The shipped-schema test pins every per-branch required group so a future drift fails loudly, keeping the structural, enum, required, and wire-keyword matrix assertions. Signed-off-by: asto <asto18089@126.com>
This was referenced Nov 5, 2026
The Moonshot chat path prefixes a digit-led registry tool name with `_` to satisfy Kimi's documented `name` pattern, but the loop only ran outbound: the response decoder deliberately keeps the wire spelling, no layer mapped `_3d_render` back to `3d_render`, and every model call for such a tool died at dispatch with "tool not available" — the tools array kept advertising the name, so the model would retry the same spelling until the turn ended. Two halves now close the loop: - Engine dispatch: after every exact and alias lookup misses, `resolve_tool_definition` falls back to the unprefixed name for the exact `_<digit>` shape the prefix produces. A genuinely underscore-led name is never stripped, and a registry holding both spellings still resolves exactly (the fallback only fires after the exact lookups missed), so approval and policy gates see the canonical registry name. - Replay: assistant tool_calls persist the registry spelling, so the Moonshot message builder re-applies the wire prefix on replayed `tool_calls` (shared `prefix_digit_led_function_name` helper with the tools-array pass). The history now matches the tools array and stays inside the documented pattern; pre-fixture histories that already carry `_` pass through untouched. Also pins the hand-copied sampling-route matcher to the config-crate reference on the shared host (the twin covers the overseas host until PR #137 widens the shared matcher; that assertion is what flips when it lands), and restores the doc comment that `sanitize_moonshot_chat_tools` had lost when the prefix function was inserted between the comment and its function. No-Issue: completes the digit-led fix in this PR's own series; the repository has issues disabled. Signed-off-by: asto <asto18089@126.com>
…oups
Two fail-open corners found by the fresh review of this series, each now
closed with a pinning test:
- `strip_mfjs_unsupported_enum_values` removed an empty `enum: []`.
An empty enum accepts nothing, so removing it silently widened an
unsatisfiable constraint into "anything goes" — exactly the
simplification of a malformed keyword the strip's own doc comment and
inline comment both forswear ("an empty or non-array enum is left for
the validator to fail closed"). The homogeneous check now treats the
empty array as vacuously homogeneous, the keyword survives the strip,
and the validator's non-empty rule fails the whole tool. A non-array
`enum` takes the same road; both shapes are pinned.
- `root_composition_constraint_note` only fired at two or more distinct
dropped required groups, so a flatten that relaxed a single required
group (one of two branches required it) loosened the contract with
zero model-facing disclosure. The note now fires for any non-empty
dropped group; the three-group exact-text pins are unchanged.
Also adds the missing boundary probes found unpinned by mutation
testing and the review lanes: the exact 75000 encoded-enum-char budget
(previous probes straddled at 74000/88000), the document-wide total
enum-value accumulation across two nodes (walle counts per node — the
constant block now discloses that scope caveat alongside the threshold
one), the 120000-byte whole-document budget (previously unprobed), and
the engine-local 64-entry structural-depth guard. The
`remove_empty_mfjs_items_schemas` doc now attributes nested `allOf` and
tuple-form `items` failures to `normalize_kimi_compatibility`, where
they actually fire, instead of validation.
No-Issue: review follow-ups for this PR's own series; the repository
has issues disabled.
Signed-off-by: asto <asto18089@126.com>
Three incompatible-tool fixtures carried the same stale comment from the superseded route-tier commit: "`pattern` itself is live-verified legal on the Kimi Code route since 2026-11-04 ... stays outside both route whitelists". The final state has one MFJS document whitelist for every Moonshot route, `pattern` fails closed everywhere (`probe_observed_keywords_fail_closed_on_every_route`), and the body's own probe declaration demotes live probes to observations that can never widen the wire. The comments now state that: `patternProperties` is simply one unlisted keyword used as the incompatible shape, and no route admits `pattern` or any other probe-observed keyword. Comment-only change; no behavior, no test churn. No-Issue: review follow-up for this PR's own series; the repository has issues disabled. Signed-off-by: asto <asto18089@126.com>
Format-only; cargo fmt on the three files touched by the review follow-ups. Signed-off-by: asto <asto18089@126.com>
This was referenced Nov 6, 2026
asto18089
commented
Nov 9, 2026
asto18089
left a comment
asto18089
left a comment
Collaborator
Author
There was a problem hiding this comment.
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; wallemodel.goSupportedKeywordscarries exactly 8 of them ($commentin 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.goDefaultValidatorConfig(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:
e3e563204exists 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.pyTOOL_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.rsdiff is tests-only;config.rsis 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_sanitize77/77). 6/7 mutations killed (prefix removal, empty-enum widening, single-group note gate, dispatch fallback death, replay no-op,.aihost 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, keepmoonshot_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 onapi.moonshot.*.
MINOR findings
- Broken citation — commit
8e2882e3dcites parent commit177dbce6, which does not exist (GitHub API 422 on both repos; invalid locally; not a force-push orphan). The message's substance is true; citecf091b5a/f5986f96or head221513a6instead. - False mechanism claim in the digit-led fix —
chat.rs:1202-1204and commit5f5eca337say "Assistant turns persist the registry spelling". Actually the session message is persisted (turn_loop.rs:2935) fromtool_usesclones (:2877-2885) beforeplan_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. - 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. - 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), andsanitize_for_responsesis 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. - Exact-75000 boundary test survives upward drift — empirically, mutating the constant to
75_001stays 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. - User docs untouched —
docs/PROVIDERS.md/config.example.tomlstill name only the.commembership host and mention sampling omission only for direct K3; nothing user-visible discloses (a) the.aimembership route or (b) that configuredtemperature/top_pis silently dropped for every model on membership routes. - K3
.aiasymmetry 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 assertsis_exact_kimi_code_k3_routeon.ai(the pin is transitive through the twin test). - Both-spellings registry → duplicate wire names — with both
1xand_1xregistered, 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. - Pin gaps —
ConflictingLiteralConstraint(enum+const disagreement → drop) has zero test coverage;defaultat root/$defs/itemsis code-verified but unpinned (the test covers properties positions only); the traversal asymmetry arounddefault(provider-neutral passes rewrite inside default payloads, MFJS-layer passes don't) is body-disclosed but undocumented in code. - 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. - Flatten disclosure gaps —
additionalProperties: falseclosure loosening and non-object-alternative narrowing are disclosed only in the PR body (no code comment, no note clause). PreservingadditionalProperties: falsewhen all object branches declare it would be provably lossless. - 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.
$refwalkers lack a step budget — walle bounds the equivalent walk (maxRefWalkSteps = 500000); the engine's terminator/depth walkers re-walk shared$reftargets 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.- Clone + walks before the size check — the whole-schema deep clone and all walks execute before the 120KB budget check (
schema_sanitize.rs:1313vs1791); an oversized schema pays full traversal before being dropped. A cheap byte pre-check onparameterswould fail fast. - 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 viato_api_tool_name, but it's the same documented-pattern rationale this PR applies to digit-led names. - 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. - 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. - 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-4186after the budget went 10→500; note-attachment separators diverge across lanes (space vs\n\n);tool_choicere-implements the digit-led predicate inline instead of reusingprefix_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_testsanchor conflict, #144 also textually overlaps #143'schat.rshunk 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
namepattern.Document evidence (verbatim quotes, fetched 2026-11-05)
docs/mfjs-spec.mdin the same repository), fetched 2026-11-05. Closing clause, verbatim: 「若官方规范中的某些概念、关键字或特性在本文档中未被提及，则表示当前 MFJS 尚未提供官方支持……我们不保证它们能在最终的结构化输出中产生稳定、符合预期的结果」— anything not listed is unsupported, so the wire carries only the listed set. Thedefaultsection, verbatim: 「default 字段为目标输出提供一个默认值……这儿不做类型检查，需要调用者自行保证其值类型内容和所在位置期望的内容相匹配」—defaultaccepts any value on any node, with no type check.namepattern^[a-zA-Z_][a-zA-Z0-9-_]{0,127}$— a leading digit sits outside it.https://api.kimi.com/coding/v1, overseashttps://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.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)type,description,default,properties,required,additionalProperties,items,enum, with$defs/$refunder their documented restrictions. The nine advisory keywords (title,$comment,$id, and the min/max bound series) are stripped instrip_mfjs_advisory_keywordsupstream of validation, never shipped; every other unlisted keyword (e.g.pattern,format,$schema,prefixItems,patternProperties) fails closed withUnsupportedKeywordand the tool is dropped — pinned per keyword byprobe_observed_keywords_fail_closed_on_every_route.enumgains same-shape scalar validation; enum-carrying violations are stripped instead of shipped, and a malformedenum([]or non-array) stays fail-closed in the validator — the strip never widens it (fixed and pinned in6d37a52cf).6d37a52cf). Undocumented approximation retained: when branches define the same property differently, the first definition wins silently.api.kimi.aihost alongsideapi.kimi.com; 6/6 live model ids on the endpoint rejecttemperature ≠ 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 onapi.kimi.aiuntil 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._prefix to satisfy the documentednamepattern — 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 assistanttool_callsso the history matches the tools array. A round trip request → model call → dispatch now completes for such tools.defaultpasses 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 adefaultvalue (e.g. a{"oneOf":[X]}-shaped default or a danglingrequiredkey in it); that is pre-existing engine behavior, not an MFJS-level type check.constliterals are translated through an internalenumchannel and are stripped before anything ships: a typed node keeps its declared type with the pin constraint released; an untyped node fails closed withMissingType; object/array consts fail closed outright. Nothing boolean/null ever reaches the wire as an enum value.conf.gosource 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'sMaxEnumItemsis per-node while the engine total is document-wide.e3e563204(absent-items termination fix, released upstream in v0.10.2) is ported faithfully, with termination tests upstream did not have.crates/tui/tests/fixtures/pptx_make_pptx_schema.json), byte-exact to the companion change below.patternare 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 in5f5eca337+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_sanitize77/77,client::550/550,core::engine::715 green / 4 ignored,turn_loop76/76.Behavior changes
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 liststypeas a single string, and the documents govern over walle's more lenient source. Previously the engine shipped the eight advisory keywords and type-checkeddefault; both behaviors are gone.additionalProperties: falseand non-object alternatives are dropped without a note..com-only pending fix(tui): replay reasoning_content for kimi k3.1 #137)._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
anyOfpptx schema inpinvou3-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
fix/kimi-k31-replay-spellings): this branch edits the topuse crate::config::{...}hunk ofcrates/tui/src/client/chat.rs(removesis_kimi_code_membership_modelandmoonshot_base_url_is_exact_kimi_codeimports); fix(tui): replay reasoning_content for kimi k3.1 #137 addsmoonshot_base_url_is_exact_direct_platformthere. The resolution is load-bearing: the union must dropis_kimi_code_membership_model(its only call site is deleted on this side; the workspace denies warnings, so keeping it fails CI) and must keepmoonshot_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.fix/xai-root-union-passthrough): both append tests at the same anchor inmod 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/v1recorded 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
f6b7eee4d,b91296cac, andebef9348areference 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.schema_sanitize::+tools::test pair failsauto_review_guardian_denial_refuses_the_call_with_a_receiptwhen 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.