Skip to content

session log: wrap remaining sidecar IChatClient call sites in SessionDiagnosticsContext.Push #920

Description

@Aaronontheweb

Background

PR #916 / #918 routes session-scoped MEL diagnostics into the canonical
per-session `session.log` based on `SessionDiagnosticsContext.SessionId`
(an AsyncLocal populated by `SessionLlmInvoker.InvokeAsync` for the main
turn path). Issue #915 proposes a Roslyn analyzer to enforce the scope at
LLM call sites going forward.

This issue tracks the existing call-site sweep that #915 will eventually
police: session-owned code that calls `IChatClient.GetResponseAsync` /
`GetStreamingResponseAsync` directly, outside of `SessionLlmInvoker`,
without first establishing the diagnostics scope.

Why this matters

Without the `Push` scope, the provider plugin's MEL log lines emitted
during the call still land in the daemon-global log but never reach the
session's `session.log`. Session-level troubleshooting is then incomplete
in exactly the paths that are most expensive to debug after the fact —
compaction, title generation, sub-agent orchestration, and memory
distillation.

Known sites (from #915)

  • `src/Netclaw.Actors/Memory/MemoryCurationActor.cs`
  • `src/Netclaw.Actors/Sessions/LlmSessionActor.cs` (any non-invoker direct calls)
  • `src/Netclaw.Actors/Sessions/Pipelines/SessionCompactionPipeline.cs`
  • `src/Netclaw.Actors/Sessions/Pipelines/SessionTitleGenerator.cs`
  • `src/Netclaw.Actors/Sessions/SessionMemoryObserverActor.cs`
  • `src/Netclaw.Actors/SubAgents/SubAgentActor.cs`

Proposal

For each call site above, wrap the `IChatClient` invocation in a
`using var scope = SessionDiagnosticsContext.Push(sessionId.Value);`
block. Prefer routing through a shared session-aware wrapper (the same
shape as `SessionLlmInvoker`) when the call belongs to a hot path with
multiple invocations, so the scope is established in one place rather
than copy-pasted at every call site.

Where the call site does not have a `SessionId` in scope, plumb it
through — every one of the listed actors and pipelines is session-scoped
by construction.

Acceptance criteria

  • All call sites listed above are covered by either an explicit
    `SessionDiagnosticsContext.Push` scope or a shared invoker wrapper.
  • A negative test exists for at least one of the major sidecar paths
    (e.g. compaction or title generation) that asserts diagnostic lines
    reach `session.log` when the path runs under a session.
  • The Roslyn analyzer from analyzer: flag session-owned chat client calls outside session diagnostics context #915, once landed, runs cleanly against the
    listed files.

Reference

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions