Skip to content

fix(runtime): populate ModelID in after_llm_call hook payload#2911

Open
kimizuka wants to merge 1 commit into
docker:mainfrom
kimizuka:fix/after-llm-call-model-id
Open

fix(runtime): populate ModelID in after_llm_call hook payload#2911
kimizuka wants to merge 1 commit into
docker:mainfrom
kimizuka:fix/after-llm-call-model-id

Conversation

@kimizuka
Copy link
Copy Markdown

@kimizuka kimizuka commented May 28, 2026

Problem

Hook handlers wired to after_llm_call read model_id and always observed
the empty string, even though pkg/hooks/types.go:177-186 documents ModelID
as populated for this event:

// ModelID identifies the model the runtime is about to call (for
// before_llm_call) or just called (for after_llm_call), in the
// canonical "<provider>/<model>" form...
ModelID string `json:"model_id,omitempty"`

Anyone writing a cost/audit/telemetry sidecar that trusts the doc and keys off
model_id ends up with no way to distinguish responses across models in the
same session.

Root cause

executeAfterLLMCallHooks (pkg/runtime/hooks.go:446-453) never set the field
on the dispatched hooks.Input:

func (r *LocalRuntime) executeAfterLLMCallHooks(
    ctx context.Context, sess *session.Session,
    a *agent.Agent, responseContent string,
) {
    r.dispatchHook(ctx, a, hooks.EventAfterLLMCall, &hooks.Input{
        SessionID:       sess.ID,
        AgentName:       a.Name(),
        StopResponse:    responseContent,
        LastUserMessage: sess.GetLastUserMessageContent(),
        // ModelID was missing — `json:"model_id,omitempty"` then elided it
    }, nil)
}

json:"model_id,omitempty" on the zero-string then elides the key from the
wire payload entirely, so handlers can't even branch on "is the field present".

The sibling executeBeforeLLMCallHooks already populates ModelID from a
parameter (pkg/runtime/loop.go:530), so this is an after-only impl gap, not a
runtime-wide one.

Fix

Thread the model identifier into the dispatch so the payload matches the
documented contract. Both callsites already have the identifier in scope:

  • pkg/runtime/hooks.go: add modelID parameter to executeAfterLLMCallHooks
    and set Input.ModelID on the dispatch.
  • pkg/runtime/harness.go:192: pass the existing local modelID.
  • pkg/runtime/loop.go:572: pass modelID.String() from the resolved
    modelsdev.ID.

No public API or wire-format addition — the field has been on hooks.Input and
documented all along; this PR just stops eliding it.

For harness agents, modelID is the harness label (e.g. claude-code) rather
than the <provider>/<model> canonical form the types.go doc describes. That's
pre-existing: the sibling executeBeforeLLMCallHooks in the harness path is
dispatched with the same modelID (pkg/runtime/harness.go:50). This PR
preserves the before/after symmetry and introduces no new asymmetry.

Tests

pkg/runtime/after_llm_call_test.go adds TestAfterLLMCallHook_PopulatesModelID,
which registers a builtin after_llm_call hook against a fixed-id mockProvider,
runs one turn, and asserts the captured Input.ModelID equals the provider's
canonical id. Verified the test fails on the unfixed tree
(expected "test/mock-model" / actual "") and passes with the fix.

Verification

  • go vet ./pkg/runtime/ — clean
  • golangci-lint run ./pkg/runtime/... — 0 issues
  • go test ./pkg/runtime/ ./pkg/hooks/... — all green

Out of scope

docs/configuration/hooks/index.md:258-259 lists per-event payload fields but
doesn't mention model_id for either before_llm_call (already populated in
code) or after_llm_call (this PR). That's a pre-existing docs gap covering
both events; a follow-up docs PR (matching the spirit of #2569) is a better
venue for catching them up together than fixing only the after-side here.

The ModelID field on hooks.Input is documented as populated for
after_llm_call (pkg/hooks/types.go:177-186), but
executeAfterLLMCallHooks never sets it, so hook handlers reading
model_id always observed an empty string.

Thread the model identifier into the dispatch so the payload matches
the documented contract. The harness path already had modelID in
scope; the loop path uses modelID.String() from the turn's resolved
modelsdev.ID.

Add a regression test that captures the after_llm_call Input via a
builtin hook and asserts ModelID equals the provider's canonical id.

Signed-off-by: kimizuka <f.kimizuka@gmail.com>
@aheritier aheritier added area/agent For work that has to do with the general agent loop/agentic features of the app kind/fix PR fixes a bug (maps to fix: commit prefix) labels May 28, 2026
@kimizuka kimizuka marked this pull request as ready for review May 28, 2026 03:03
@kimizuka kimizuka requested a review from a team as a code owner May 28, 2026 03:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/agent For work that has to do with the general agent loop/agentic features of the app kind/fix PR fixes a bug (maps to fix: commit prefix)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants