refactor(agent-core-v2): rebuild the model wire layer on the kosong architecture#1970
Conversation
🦋 Changeset detectedLatest commit: 4c0ebad The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 336c43e089
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ? this.contextSize.get().measured | ||
| : undefined, | ||
| }); | ||
| const requester = this.modelCatalog.getRequester(resolved.modelAlias); |
There was a problem hiding this comment.
When model/provider config changes after prepareTurnConfig() has cached a turn, later requests in that same turn still use the cached resolved params but this line re-resolves the ModelRequester from the live catalog. Since the catalog invalidates on model/provider changes, a multi-step turn can switch endpoints/capabilities mid-turn while telemetry and budget data remain from the old model; cache the requester/model in TurnRequestConfig as the previous model snapshot did.
Useful? React with 👍 / 👎.
| for (const [modelId, record] of Object.entries(models)) { | ||
| if (record.provider !== undefined && record.provider in excludedProviders) { | ||
| excludedModels[modelId] = record; |
There was a problem hiding this comment.
For a static provider whose models use the structured providerId key, this exclusion misses those models because it only checks record.provider. On the default startup/interval all-provider refresh, the static provider is hidden from the refresh host but its providerId-bound models are not merged back as excluded models, so the orchestrator's whole-section model write can drop the user's static model/default selection; match record.providerId ?? record.provider here.
Useful? React with 👍 / 👎.
- merge the seven kimi trait modules into kimi.contrib.ts as two trait objects: kimiOpenAITrait (all native-transport hooks) and kimiAnthropicTrait (thinking only) - move bases/* implementations into per-protocol directories (openai/, anthropic/, google-genai/) and rename openai.contrib.ts to openai-legacy.contrib.ts - add per-directory index.ts registration barrels (import = registration) and exempt them in check-domain-layers.mjs alongside *.contrib.ts
- consolidate shared model types (ModelOverrides, CompletionBudgetConfig/Params, ResolvedModelAuthMaterial, ThinkingDefaults/ModelThinkingMetadata) into kosong/model/model.types.ts and drop modelOverrides.ts - rename L2 request types to the ModelRequest* prefix: LLMEvent -> ModelRequestEvent, LLMRequestInput -> ModelRequestInput, LLMCallParams -> ModelRequestParams - extract ModelRequestTiming to replace three duplicated copies of the stream-timing shape - rename L3 llmRequester types with the Agent prefix to match IAgentLLMRequesterService (AgentLLMRequestOverrides/Finish/Task/Source/ PartHandler/LogFields) - delete the unused LLMRequestParams type
- merge IModelCatalogService's enumeration surface (listModels / listProviders / getProvider / setDefaultModel and the wire shapes) into IModelCatalog; delete kosong/catalog/modelCatalog.ts - move the remote refresh path to the new IProviderDiscoveryService (discovery.ts + discoveryService.ts, renamed from catalog/modelCatalogService.ts) - relocate configSection / errors to discoveryConfigSection.ts / errors.ts; DEFAULT_MODEL_SECTION now lives in kosong/model/model.ts - drop the L3 catalog layer from check-domain-layers.mjs - kap-server routes / refresh scheduler / channelRegistry follow the split; klient renames the contract modelCatalogService to modelResolver and adds providerDiscovery; kimi-inspect reads via IModelCatalog - modelRequesterImpl: drop the streamedAnyPart backfill, onMessagePart already delivers every part - tests: remove the app/modelCatalog and kosong/catalog suites, add the kosong/model catalog and discovery suites
- add trimTrailingUndefined helper so optional trailing args no longer cross the wire as null in http/ipc transports, which defeated server-side default parameters - add model-requester-boundary smoke probe for ChatProvider error wrapping behavior against real config and a local stub - add kimi-select-tools smoke probe verifying the kimi-only wire encoding of dynamic tool declarations - extend smoke.ts with a models set/get/delete round-trip, catalog list assertions, and update AGENTS.md with the new scripts
Method-shorthand members on OpenAIChatCompletionsHooks tripped typescript-eslint(unbound-method) at every extraction site (`const hook = this._hooks?.convertMessage` and friends), failing the repo-wide lint job. Every implementation is a plain closure composed by openaiHooks.ts, so declare the members as function-typed properties, which matches the actual semantics and clears the four errors.
- agent-core-v2: point vacuousContent's ContentPart import at kosong/contract - kap-server: rewrite the transcript test seed as IModelCatalog (IModelResolver is gone) - klient: inline onceEvent/waitFor after the http transport helpers were dropped - kimi-inspect: remove useLiveEvent from ModelCatalogView; catalog polls on a slow interval
f5f309a to
e9d2271
Compare
1 participant
Related Issue
No linked issue — this PR lands the long-running
kosongrefactor branch for the experimental v2 engine, developed internally. The problem statement is below.Problem
The v2 engine's model wire layer had accreted into per-provider dialect classes with vendor-name gates sprinkled through engine code. Adding or fixing a provider meant copying adapter code and hunting down vendor checks outside the layer that owned them; request intent (prompt cache key, thinking effort, sampling) reached providers by cloning whole model objects; and a never-functional
[platforms]config section plus a redundantprovider.platformIdfield made credential resolution a three-way guessing game. Meanwhile the branch had drifted behindmain, which deleted the/api/v2RPC surface and added per-turnthinking_efforttelemetry on the old skeleton.What changed
1. Land the kosong layered wire architecture
kosong/contract/): provider-agnostic wire types — messages, tools, usage, errors, capabilities, request trace.kosong/protocol/): a trait-based protocol registry — declarative traits (auth, streaming, thinking, media) compose into per-protocol bases, replacing handwritten dialect classes.kosong/provider/): per-protocol provider definitions as data, not code; Kimi is one composed definition among peers.kosong/model/): materialization, two-layer model → provider credential resolution, completion-budget folding, thinking-effort resolution, and a read-only catalog over the same materialization the runtime uses.2. Migrate the engine and peripherals onto kosong
llmRequester, loop, profile) now drive aModelRequesterresolved fromIModelCatalog; per-turn request intent flows as request parameters instead of cloned model objects. The oldapp/llmProtocollayer is deleted.vertexaiprotocol is folded intogoogle-genaiviaproviderOptions; the unused[platforms]config section andprovider.platformIdare removed.3. Model resolution inspection and connectivity ping
IModelCatalog.inspectreports the three raw config layers plus the resolved runtime view with per-field provenance (config / override / builtin / env / synthesized), andpingprobes live endpoint connectivity — exposed for internal debugging tools (kimi-inspect's Model Catalog view).4. Test coverage for provider edge behavior
undefinedargs are trimmed so server-side defaults are no longer defeated bynullon the wire.5. Merge
main(transcript layer,/api/v2removal, thinking_effort telemetry)/api/v2RPC surface: the klient http transport and the kap-server channel whitelist are gone; kimi-inspect keeps its Model Catalog view on the/api/v1/debugreflection surface and the unified transcript layer.thinking_efforttelemetry on the kosong requester:prepareTurnConfigsnapshots the turn's model/effort/system-prompt at the turn boundary so loop telemetry and every request in the turn share one configuration.Checklist
gen-changesetsskill, or this PR needs no changeset (4 changesets on the branch: kosong architecture as minor, abort-retryable / prompt-cache-key / model-inspection as patch).gen-docsskill, or this PR needs no doc update (v2 engine is experimental and not user-facing; no CLI behavior change).