refactor(agent-core): slim llm request log line - #190
Conversation
🦋 Changeset detectedLatest commit: 3df54e5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
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 |
f3fe04f to
913b584
Compare
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 913b584cd5
ℹ️ 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".
| "@moonshot-ai/kimi-code": patch | ||
| --- | ||
|
|
||
| Slim the `llm request` diagnostic log line: merge `turnId`/`step` into a single `turnStep` field and `attempt`/`maxAttempts` into `attempt`, and drop the `messageCount`/`toolCallCount` fields. The deduplicated `llm config` line is unchanged. |
There was a problem hiding this comment.
Correct stale note about llm config fields
The changeset says the deduplicated llm config line is unchanged, but this commit also routes that log through the new buildLlmRequestContext shape, and the updated test now expects turnStep instead of the previous turnId/step fields for llm config. Users relying on the release note will miss that diagnostic field rename, so either keep llm config backward-compatible or update this note to describe the changed context fields.
Useful? React with 👍 / 👎.
913b584 to
9569f9e
Compare
Merge turnId/step into a single `turnStep` field ("0.1") and
attempt/maxAttempts into `attempt` ("2/3"), and drop the
messageCount/toolCallCount fields. The per-request `llm request`
line goes from up to 8 fields down to ~3; the `llm config` line
(including thinkingEffort, logged for all providers) is unchanged.
9569f9e to
3df54e5
Compare
|
Codex Review: Didn't find any major issues. Delightful! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
Related Issue
No prior issue. Opening directly per CONTRIBUTING's "can open a PR directly" path — this is a small, focused refactor (~94 lines, under the ~100-line threshold) that touches diagnostic log output only, not product-user-visible behavior. Problem described below.
Problem
The high-frequency
llm requestdiagnostic log line carried up to 8 fields per request (turnId,step,attempt,maxAttempts,estimatedInputTokens,messageCount,toolCallCount,partialMessageCount), making it wide and noisy.What changed
Slim the per-request
llm requestline down to ~3 fields:turnId+step→turnStep(e.g."0.1")attempt+maxAttempts→attempt(e.g."2/3", only emitted on retries)messageCountandtoolCallCountThe deduplicated
llm configline is unchanged (still includesthinkingEffortfor all providers). The sameturnStep/attemptformatting is applied to thellm request failedline in the retry path for consistency.Checklist
turn.test.tsanderror-paths.e2e.test.tsto assert the slimmed field shape.)gen-changesetsskill, or this PR needs no changeset. (Added.changeset/slim-llm-request-log.md— patch bump for@moonshot-ai/agent-core+@moonshot-ai/kimi-code.)gen-docsskill, or this PR needs no doc update. (Internal log format; no user-facing docs affected.)