feat(agent-core-v2): record compaction droppedCount in wire traces and rename select_tools capability#1707
Conversation
…st wire ops - add per-attempt droppedCount to the llm.request source logFields so replays can see how much history each retry round blinded - cover compaction/loop request events in the full-compaction test
…y_loaded_tools - rename the ModelCapability bit and catalog field across capability.ts, catalog.ts, modelResolverService and the toolSelect gate - update toolSelect flag description wording to match - update all affected tests and the test harness capabilityNames helper
🦋 Changeset detectedLatest commit: 30bf4e9 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: 30bf4e927e
ℹ️ 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".
| declared.has('dynamically_loaded_tools') || | ||
| detected.dynamically_loaded_tools === true, |
There was a problem hiding this comment.
For models configured before this rename with capabilities = ['select_tools'] (or KIMI_MODEL_CAPABILITIES=select_tools), this resolver now leaves dynamically_loaded_tools false because it only recognizes the new string; then AgentToolSelectService.enabled() never opens the progressive-disclosure gate even though the model was explicitly configured for it. Either keep declared.has('select_tools') as a compatibility alias or treat this as a breaking config migration rather than a patch.
Useful? React with 👍 / 👎.
| // Per-attempt count of messages dropped by overflow/empty | ||
| // shrinks so far; recorded on the llm.request wire op so a | ||
| // replay can see how much history each retry round blinded. |
There was a problem hiding this comment.
packages/agent-core-v2/AGENTS.md says comments in this tree must live only in the top-of-file block and never beside statements. This new inline explanation violates that package rule; move any needed rationale into the file header or let logFields: { droppedCount } stand on its own.
Useful? React with 👍 / 👎.
1 participant
Related Issue
No linked issue. This PR closes two parity gaps found in a v1→v2 port audit of
packages/agent-core-v2againstpackages/agent-core(v1 references: #1448 / commit65d30177, and #1488 / commit7bd29ab0).Problem
Two v1 behaviors were missing or divergent in the v2 engine:
droppedCounthad no producer. v1's full compaction passesrequestLogFields: { kind: 'compaction', droppedCount }on every summarizer request, so eachllm.requestrecord inwire.jsonlcarries how many history messages had been dropped by overflow / empty-response shrink retries so far. v2 already had the payload field (llmRequestOps) and the reader (llmRequesterService), but the compaction request never set the field, so rebuilding or debugging a compaction retry sequence fromwire.jsonllost that signal.ModelCapabilitybitselect_tools→dynamically_loaded_tools(refactor(kosong): rename select_tools capability to dynamically_loaded_tools #1488), but v2's localllmProtocolfork (capability / catalog / model resolver / toolSelect gate / flag description) still used the old spelling: a config declaringdynamically_loaded_toolstakes effect on v1 but is silently ignored on v2, and future ports from kosong would hit a naming mismatch.What changed
1. Record
droppedCounton full-compactionllm.requestwire ops (48bd6abbb)fullCompactionServicenow passeslogFields: { droppedCount }on the compaction request source; the existinglogFieldsForSource→recordRequestpath writes it into thellm.requestpayload (kind: 'compaction'tagging was already in place viarequestKind: 'full_compaction').0on the first attempt), matching v1 semantics.llm.requestrecords carry(kind, droppedCount)=('compaction', 0),('compaction', 2),('loop', undefined).2. Rename the
select_toolscapability todynamically_loaded_tools(08bd2ed48)llmProtocolcapability and catalog, the model resolver's declared/detected merge, and the toolSelect gate, mirroring v1's kosong change.dynamically_loaded_toolswith no alias kept — no catalogued model or shipped configuration uses the capability, so there is nothing to migrate (same call as v1).select_toolstool name and thetool-selectflag id stay; the flag description now says "declares dynamically loaded tools", matching v1.Verification:
pnpm typecheck(coverssrc+test) is clean; 208 tests across the 7 affected test files pass locally.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.