Repository navigation
Preserve episode identity, caller lineage, and unified session metadata - #67518
Conversation
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft.
|
|
🔎 PR Code Quality Reviewer is reviewing code quality for this pull request... |
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality Sentinel skipped because pre-fetch PR data was unavailable: unable to fetch test file diff
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully! Lean already. Ship.
|
There was a problem hiding this comment.
Note
Copilot was unable to run its full agentic suite in this review.
Copilot review overview
4 open findings
fields(data.episode, ...)returns an empty string… · NewStartedAtandUpdatedAtare zero prevents falling back to… · Newlen(candidates) != 1, so using aforloop to return the single… · NewAwContextfields don’t have the inline comments that the surrounding… · New
What changed in this PR
Adds canonical episode lineage fields end-to-end (schemas, Go rollups, JS session normalization/rendering, and OTLP attribute behavior) so logs and unified sessions can preserve runner-owned episode identity across workflow hops.
Changes:
- Extend log/audit schemas and Go
AwContextto include canonical episode lineage fields (episode_id,hop_id,parent_hop_id, etc.). - Rework Go episode aggregation to prefer canonical lineage, avoid conflicting merges, and centralize rollup logic in an accumulator.
- Project/render episode lineage into unified session payloads and update OTLP attribute handling + tests/docs.
| File | Description |
|---|---|
| schemas/logs.schema.json | Adds canonical episode lineage fields to logs schema. |
| schemas/logs-jsonl.schema.json | Adds canonical episode lineage fields across JSONL schema sections. |
| schemas/audit.schema.json | Adds canonical episode lineage fields to audit schema. |
| pkg/cli/logs_models.go | Extends AwContext with canonical episode lineage fields. |
| pkg/cli/logs_episode.go | Reworks episode grouping/edge building to respect canonical lineage; refactors rollup. |
| pkg/cli/logs_episode_rollup.go | New accumulator-based episode rollup implementation. |
| pkg/cli/*.go tests | Adds tests asserting canonical context preservation and conflict handling. |
| actions/setup/js/unified_session_payload.cjs | Projects episode lineage from trusted workflow context into unified payload. |
| actions/setup/js/unified_session_render.cjs | Renders episode lineage in session summaries. |
| actions/setup/js/send_otlp_span.cjs | Uses resolveParentHopId for hop parent attribute emission. |
| docs/* | Documents episode lineage behavior and updated CLI examples/schema. |
🧠 Review effort: Lite
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
| return ( | ||
| fields(data, ["engineId", "model", "requestedModel", "modelRouting", "triggerType", "cliVersion", "awfVersion", "mcpgVersion", "agentVersion", "workflow", "repository", "runId"]) + | ||
| " " + | ||
| fields(data.episode, ["episodeId", "hopId", "parentHopId", "originEvent", "rootRepo", "rootWorkflowId", "rootRunId"]) | ||
| ); |
| if run.StartedAt.IsZero() && run.UpdatedAt.IsZero() { | ||
| return | ||
| } | ||
| if !run.StartedAt.IsZero() && !run.UpdatedAt.IsZero() && run.UpdatedAt.After(run.StartedAt) { | ||
| acc.duration += run.UpdatedAt.Sub(run.StartedAt) | ||
| } else if processedRun.Run.Duration > 0 { | ||
| acc.duration += processedRun.Run.Duration | ||
| } |
| for _, parent := range candidates { | ||
| return EpisodeEdge{ | ||
| SourceRunID: parent.RunID, | ||
| TargetRunID: run.RunID, | ||
| EdgeType: edgeType, | ||
| Confidence: confidence, | ||
| Reasons: append([]string(nil), reasons...), | ||
| SourceRepo: parent.Repository, | ||
| SourceRef: parent.WorkflowPath, | ||
| EventType: parent.Event, | ||
| }, true | ||
| } | ||
| return EpisodeEdge{}, false |
| EpisodeID string `json:"episode_id,omitempty"` | ||
| HopID string `json:"hop_id,omitempty"` | ||
| ParentHopID string `json:"parent_hop_id,omitempty"` | ||
| OriginEvent string `json:"origin_event,omitempty"` | ||
| RootRepo string `json:"root_repo,omitempty"` | ||
| RootWorkflowID string `json:"root_workflow_id,omitempty"` | ||
| RootRunID string `json:"root_run_id,omitempty"` | ||
| Time string `json:"time,omitempty"` // ISO 8601 timestamp of the dispatch | ||
| Actor string `json:"actor,omitempty"` // GitHub actor that triggered the calling workflow | ||
| EventType string `json:"event_type,omitempty"` // GitHub event name of the calling workflow | ||
| ItemType string `json:"item_type,omitempty"` // Kind of triggering item: "issue", "pull_request", "discussion", "check_run", "check_suite", or "" | ||
| ItemNumber string `json:"item_number,omitempty"` // Number (issue/PR/discussion) or database id (check_run/check_suite) of the triggering item | ||
| CommentID string `json:"comment_id,omitempty"` // ID of the triggering comment or review; empty when not a comment/review event |
🏗️ Design Decision Gate — ADR RequiredThis PR makes significant changes to core business logic (426 new lines across 📄 Draft ADR: intended path
📄 Draft ADR-67518 (copy to
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Skills-Based Review 🧠
Applied /codebase-design and /tdd to the episode-lineage core (pkg/cli/logs_episode.go, logs_episode_rollup.go) and the JS lineage projection (aw_context.cjs, unified_session_payload.cjs). The refactor into episodeAccumulator methods and the canonical-episode union-find logic are clean and well-covered; test coverage for conflicting lineage, missing hops, and shared-run self-edges is strong.
📋 Key Themes & Highlights
Key Themes
- Inconsistent trimming in
classifyEpisode'sstandalonedetection vs. the trimmedcanonicalEpisodeIDhelper used two lines above it — flagged inline. - Four pre-existing Copilot review comments (trailing-space rendering bug, duration-fallback early return, unreachable-looking
forloop, missing field comments) remain valid and unaddressed; recommend resolving those alongside this round.
Positive Highlights
- ✅
episodeAccumulatorextraction intologs_episode_rollup.gocleanly separates aggregation concerns per/codebase-design. - ✅ Canonical-identity union-find (
groupCanonicalEpisodes/buildEpisodeEdges) is well tested against conflicting lineage, reversed/partial selections, and shared-run self-edges (TestBuildEpisodeDataPreservesCanonicalIdentityAcrossSelections,TestBuildEpisodeDataRejectsConflictingCanonicalLineage). - ✅
resolveParentHopIdconsolidation inaw_context.cjs/send_otlp_span.cjsremoves duplicated parent-resolution logic and is backed by table-driven tests.
Note: JS test suite (vitest) could not be run locally in this environment due to a registry/cert restriction; findings are based on static review of the diff and surrounding code.
@copilot please address the review comment above.
🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · copilot · sonnet50 · 325.7 AIC · ⌖ 14.5 AIC · ⊞ 10.3K
Comment /matt to run again
| kind := "dispatch_workflow" | ||
| if run.Event == "workflow_call" { | ||
| kind = "workflow_call" | ||
| } else if run.AwContext.RunID == strconv.FormatInt(run.RunID, 10) && run.AwContext.ParentHopID == "" { |
There was a problem hiding this comment.
[/codebase-design] classifyEpisode's standalone detection compares run.AwContext.ParentHopID and run.AwContext.RunID without trimming, while canonicalEpisodeID (used just above) trims the same context via strings.TrimSpace. A whitespace-only parent_hop_id (e.g. " ") would be treated as non-empty here, misclassifying a root hop as dispatch_workflow instead of standalone.
💡 Suggested fix
Use strings.TrimSpace(run.AwContext.ParentHopID) == "" (and trim RunID similarly) so whitespace-only lineage fields can't flip the classification, matching the trimming already applied by canonicalEpisodeID.
@copilot please address this.
|
🎉 This pull request is included in a new release. Release: |
Why
Episode reporting dropped canonical lineage fields, split partial selections of the same episode, and recorded a grandparent instead of the immediate caller across multiple workflow hops. Unified agent sessions also omitted the available episode metadata.
Approach
workflow.info.data.episodeobject inusage/aw_session.jsonl, with type declarations, generated schema, text rendering, and specification updates.Compatibility and boundaries
Legacy contexts retain their existing behavior. CLI rollups remain run-level and cover only selected runs; this does not introduce per-hop graph nodes or full-lifetime totals. Unified-session metadata preserves the recorded caller's identifiers rather than synthesizing a current hop. Legacy call IDs alone do not create canonical episode metadata. The session file-format version remains
1, and unrelated caller context is excluded.These changes add no GitHub API requests.
Validation
npm run typecheck, formatting, generated-schema freshness, and workflow drift checks passed.BASE_REF=HEAD PATH="/opt/homebrew/bin:$PATH" make agent-report-progress.The earlier combined validation encountered a shared-host golangci-lint lock; its non-test checks passed on retry with an isolated temporary directory.