feat(worker): self-trace the digest-summary LLM call - #2075
Open
GabrielDrapor wants to merge 5 commits into
Open
feat(worker): self-trace the digest-summary LLM call#2075GabrielDrapor wants to merge 5 commits into
GabrielDrapor wants to merge 5 commits into
Conversation
Deviations from the plan's test sketch, forced by the real digest-summary.ts
signature (generateDigestSummary(input: DigestSummaryInput, cfg:
DigestSummaryModelConfig), returning {summary, usage} | null):
- projectId has no home in either existing param; added to
DigestSummaryModelConfig (cfg) since detector-digest-processor.ts already
has it in scope, and threaded it through the one call site.
- windowStart/windowEnd are Date, not epoch ms; digestTraceId is called with
their .getTime().
- The digest flow has no per-finding ids at this point (only per-detector
counts and sampled summaries) — root metadata carries detector
name/findingCount instead of the plan's finding_ids.
- Test asserts on out?.summary since the function can return null, and its
input includes a detector with a non-empty sampleSummaries entry (an empty
set short-circuits buildDigestSummaryPrompt before any LLM call).
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018DopjNH21gLaDtWeeMDEHX
PR overviewThis pull request routes digest-summary model calls through the worker’s self-tracing and traced-completion infrastructure while updating self-trace metadata types and callers. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Scanned with Semgrep · TruffleHog · Trident review. View in Trident Fixed/addressed: 0 · PR risk: 0/10 |
Contributor
There was a problem hiding this comment.
All reported issues were addressed across 5 files
Architecture diagram
sequenceDiagram
participant Scheduler as Digest Scheduler
participant Processor as Worker Digest Processor
participant Summary as digest-summary
participant Config as Model and API Key Resolver
participant Trace as withSelfTrace
participant LLM as tracedComplete / LLM Provider
participant TraceRoot as Traceroot
participant DB as Trace and ai_messages Storage
Note over Scheduler,DB: Current digest-summary runtime flow
Scheduler->>Processor: Flush digest job for project and time window
Processor->>Summary: generateDigestSummary(input, projectId, model config)
Summary->>Config: Resolve provider config, model, and detector API key
Config-->>Summary: Model and authenticated provider access
Summary->>Summary: Compute trace id = sha256(projectId:windowStart:windowEnd)[0:32]
Summary->>Trace: withSelfTrace(traceId, projectId, name=digest-summary, metadata)
Note over Summary,Trace: Metadata includes kind=digest and digest finding/window context. Source remains detector because the caller is the worker
Trace->>Trace: Establish self-trace scope
Trace->>TraceRoot: Create trace root with digest-summary name and project attribution
TraceRoot-->>Trace: Active trace scope
Trace->>LLM: tracedComplete(model, prompt, messages, tools, timeout signal)
LLM->>DB: Record LLM usage in ai_messages
LLM->>TraceRoot: Emit model call span and response data
LLM-->>Trace: Completion response
Trace-->>Summary: Successful traced result
Summary->>Summary: Parse submit_digest_summary tool result
Summary-->>Processor: Digest summary or null
Processor-->>Scheduler: Digest flush completed
alt LLM timeout, abort, or self-trace setup failure
LLM-->>Trace: Aborted/error response
Trace-->>Summary: Error result or plain execution fallback
Summary-->>Processor: null or digest failure
else LLM/provider failure
LLM-->>Trace: Error
Trace-->>Summary: Propagate traced error
Summary-->>Processor: Digest generation failure
end
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
…ised Review asked for finding_ids on the digest's self-trace metadata so the run can be tied to what it summarised. No finding id reaches this code: the digest is built from the window-summary endpoint, which returns finding_count, sample_summaries and sample_trace_ids — counts and text, never finding rows. Adding ids would mean widening that endpoint. The sampled trace ids answer the same question and are already in hand, so they are threaded through to the metadata instead. They are also the more useful link: a trace id is something a reader can open, and it matches how an RCA trace records its scanned_trace_id. Co-Authored-By: Claude <noreply@anthropic.com>
…race Reverts 4099b1d. Nothing on the stack tip reads the ids off the trace metadata, and the same value already travels as DigestEntry.latestTraceId. Keep the metadata to what the trace list shows: kind, window and per-detector counts. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015DXLChvn8HPA42wsuDUFYo
digestTraceId hashed (project, window) so a re-flushed window reused one trace id. Re-flush is reachable (the enqueue dedupe key lives 6h; a stalled job is re-run after a crash), and each flush still makes its own LLM call with fresh span ids, so the second flush appended a second root and a second LLM span under the same trace. Nothing recomputes the id from the window, so draw a random one per run — two flushes, two traces, matching the detector path. Also fix the DigestSummaryModelConfig comment: this root is a detector-source trace (the worker authenticates with INTERNAL_API_SECRET), not an agent one. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015DXLChvn8HPA42wsuDUFYo
… digest DetectorRunMeta | GenericSelfTraceMeta with an `in`-probe discriminator was two shapes for one thing: the emitter only ever needed a trace id, a project, a root name and root metadata. Collapse to SelfTraceMeta and have the detector call site build its own traceId/name/metadata, as the digest already did. The try/catch around the id derivation only guarded meta.runId.replaceAll; with no derivation left, the emitter passes meta straight to observe(), whose failures are already covered by the "observe fails before reaching fn" path. The "unusable meta" test went with it — there is no throw source left to test. Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015DXLChvn8HPA42wsuDUFYo
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
notifications/digest-summary.tscalls pi-ai'scomplete()directly, unlike the judge which goes throughtracedComplete. It is the one model call in the product with no trace at all — only a usage row inai_messages.Routes it through the worker's existing
tracedComplete+withSelfTrace(the judge's path, already shipping) with root namedigest-summary, trace idsha256(project:window_start:window_end)[:32], and metadata{ kind: "digest", finding_ids }.sourcestaysdetector— source is derived from the authenticated caller, and this is the worker. No new emitter, no new secret.A test asserts
digest-summary.tsdoes not callcomplete()directly; re-adding it is the regression this exists to prevent.Independent of the agent self-trace stack —
tracedCompleteandwithSelfTraceare already onmain, so this can land on its own.Closes #2066. Related to #2058.