✨ feat(timeline): complete the lifecycle panel — journeys, all stages, dedupe, persistence - #5670
Conversation
…, dedupe, persistence The Lifecycle Timeline panel rendered only re-stamped enumeration noise: 4 of 6 stages had no producer, the 500-raw-event ring flooded every eval cycle (~14min of history under a claimed 6h window), and restarts wiped it. Finish the feature instead of removing it (#5656): Store (pkg/timeline): events are now keyed by repo#number+kind into JOURNEYS — one entry per work item with per-stage FirstAt/LastAt/Count/ agent/attrs and a derived Current stage (merged sticky-terminal; blocked holds until later progress). Re-enumeration refreshes a timestamp instead of appending. Ring is sized for journeys (500 journeys, LRU by LastAt) and persists to /data/lifecycle-timeline.json with the atomic-persist idiom (unique temp + fsync + rename, per the post-#5625 mutation ledger), throttled for non-terminal floods and immediate for pr_opened/merged/blocked; corrupt files are moved aside, never fatal. FleetHealth now reports CoveredMs — how much history actually backs the counts — alongside the requested window. Producers, all riding existing paths (no new polling, no new loops): - classified: the scheduler's classifier pass (BuildKickMessages / BuildAgentMessageFromLastActionable) records lane/tier/model at the moment lane routing decides them (Scheduler.SetLifecycleRecorder). - pr_opened: the pr-request watcher's typed PROpenedHook plus the attribution audit stream (agent_pr_created) — deduped by design. - merged: every pr_merged attribution audit entry — both automerge sweep paths and MergePR (dashboard queue + merge watcher) — via the audit sink bridge in cmd/hive/lifecyclewire.go. - blocked: the fix-loop escalation (needs-human) records the real blockage with fix attempts and failing checks, not just hook annotations. Refs are normalized to the short repo#number form the enumeration producer already uses, so all stages of one item share one journey. Dashboard: /api/lifecycle-timeline serves {journeys, fleet}; the ACMM filter now gates whole journeys. Panel B renders one row per item with stage chips (enum → class → kick → pr → merged/blocked), current stage colored by the existing kind→color mapping, and an honest coverage label from fleet.coveredMs. EnableLifecyclePersistence mirrors the session-persistence wiring. pkg/retro keeps working: applyTimeline reads the journey's kicked-stage Count (kick cardinality preserved) and stage attrs for PR refs/state; its existing Reconstruct expectations pass unchanged. Tests: store dedupe/transitions/eviction/persistence round-trip/honest coverage/race, producer wires (classified, pr_opened, merged, blocked via a real runEscalationSweep pass), journey API contract, and markup-pin tests for the new panel per the #5479 style. Fixes #5656 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
98ba5d2 to
d039b22
Compare
clubanderson
left a comment
There was a problem hiding this comment.
Independent verification against the v4 source. Verdict: APPROVE — safe to merge.
Verified:
- Dedupe semantics: Record keys by IssueRef+Kind, refreshing LastAt/Count/attrs — the every-cycle enumeration sweep can no longer flood (50 re-records → 1 journey, Count 50, pinned). deriveCurrent's rules (merged sticky-terminal, blocked holds only until later progress, re-enumeration never regresses the stage) are each pinned by a test.
- Journey identity actually converges: enumerated uses Issue.Repo (config repos are short names, or org-qualified only for cross-org repos), classified uses the same worksource Key over the same field, and the audit/hook/escalation sources go through timelineRef which strips only the project org — so 'repo=owner/repo' from the automerge sweep and short 'repo' from the pr_request watcher both land on the enumeration's key. Cross-org repos keep their qualifier on both sides. All four stage-splitting scenarios are covered by TestTimelineRef.
- Producers are wired where claimed, all on existing paths: SetAttributionAudit callback (covers agent_pr_created + all three pr_merged emitters — both automerge sweep paths at automerge_sweep.go:571/707 and MergePR at pullrequest.go:208), the typed SetPROpenedHook, the scheduler's classify pass in BuildKickMessages/BuildAgentMessageFromLastActionable, and runEscalationSweep's newly-escalated path only. parseAuditDetail's ', ' split matches AuditDetail's join. No new polling or goroutines.
- pkg/retro compatibility: ByIssue/Recent survive as synthesized views; KicksReceived now reads Stage.Count (true cardinality, which the collapsed event view can't carry), ClaimedAt from FirstAt, drift-pause and pr_state scans made deterministic by LastAt ordering. No other consumer of the removed ring API exists outside the package (hub/timeline.go is an unrelated store).
- Persistence: atomic idiom done right (unique temp + chmod + write + fsync + close + rename, temp removed on every failure path), corrupt file moved aside instead of refusing startup (correct for telemetry, unlike the mutation ledger), throttled 15s for non-terminal floods with immediate persist for pr_opened/merged/blocked. Everything runs under the store's own mutex with no re-entrancy.
- Honest windowing: CoveredMs is derived and the UI test pins that the label uses fleet.coveredMs ('of recorded history') rather than claiming the requested window; the ACMM-filtered fleet counts are re-derived over the full filtered set, not the limit-truncated page.
- Markup pins: both UI tests hold against the shipped index.html (journey rows, fixed stage axis through the existing v4KindClass mapping, old lc-events list gone), and the maxTimelineEnumeratePerCycle bump is now justified by dedupe rather than papering over eviction.
One observation, not a blocker: #5656's headline proposal was removal, with completion as the scoped alternative — this PR takes the alternative, and having traced the producers I think that's the defensible call; the panel now shows real outcomes instead of noise. Worth the operator knowing the decision point existed.
The pre-merge coverage gate flagged pkg/timeline at 88.2% against its 90% floor after the journey-store rewrite. Cover the real behavior the rewrite introduced, not ceremony: - out-of-order producers: a late-arriving older event extends the stage/journey span backwards (audit sink vs typed hook ordering) - deriveCurrent fallback: unknown kinds resolve to the most recently touched stage (deterministic tie-break), and a later UNKNOWN kind does not clear a block — only known progress stages do - over-cap load: a file from a larger store loads into a smaller one with LRU eviction, and persisted entries with no ref/stages are skipped instead of resurrected as empty rows - unreadable-but-existing file surfaces an error while persistence stays enabled and recovers on the next save - persist failure (missing /data dir on dev machines) never drops in-memory journeys, throttles its logging, and the dirty state is retried and lands once the path becomes writable - FleetHealth clamps skewed future timestamps; Recent(n) truncates pkg/timeline: 88.2% → 95.6%, -race clean. Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
Fixes #5656
The Lifecycle Timeline panel ("Panel B") rendered only re-stamped enumeration noise: 4 of its 6 stages never had a producer, the two that existed flooded a 500-raw-event ring every eval cycle (~14 minutes of retained history under a claimed 6h window), and a restart wiped everything — so the counters read
0 merged / 0 blockedon hives that merge constantly. Per the operator's call on #5656, this finishes the feature rather than removing it.Store: journeys, not raw events (
pkg/timeline)repo#number+ kind into journeys — one entry per work item with per-stagefirstAt/lastAt/count/agent/attrs and a derivedcurrentstage. Re-enumeration refreshes a timestamp on the existing journey instead of appending 50 rows per cycle.mergedis sticky-terminal;blockedholds until later progress clears it; otherwise the furthest progress stage wins.MaxJourneys = 500, LRU-evicted bylastAt) — weeks of fleet history where the raw ring held minutes./data/lifecycle-timeline.jsonwith the repo's atomic-persist idiom (unique temp + fsync + rename, per the post-[architect] refactor: crash-safe persist for pkg/convergence/mutation Ledger (unique temp + fsync) #5625 mutation ledger). Throttled (15s) for non-terminal refresh floods, immediate forpr_opened/merged/blocked. A corrupt file is moved aside to.corruptand never refuses startup — this is telemetry, not ownership state.FleetHealthgainscoveredMs: how much history actually backs the counts, so the roll-up can never again claim a 6h window over a 14-minute ring.Producers — all riding existing paths (no new GitHub polling, no new goroutines)
enumeratedcmd/hive/main.gorecordEnumeratedIssues— cap raised to the endpoint's journey limit now that dedupe removed the eviction-flood concernclassifiedpkg/schedulerBuildKickMessages/BuildAgentMessageFromLastActionable→recordClassifiedvia newSetLifecycleRecorderkickedcmd/hive/main.gorecordKickpr_openedPROpenedHook+ the attribution audit stream (agent_pr_created)cmd/hive/main.gohook wrapper +cmd/hive/lifecyclewire.gorecordLifecycleFromAudit(deduped by design)mergedpr_mergedattribution audit entry: both automerge sweep paths andMergePR(dashboard queue + merge-watcher merges humans drive through the hive)cmd/hive/main.go→recordLifecycleFromAuditblockedneeds-human), carrying fix attempts + failing checks — not just hook annotationsrunEscalationSweep→recordBlockedRefs are normalized to the short
repo#numberform the enumeration producer already uses (verified against the live snapshot), so all stages of one item share one journey. The celtriggerKindPROpenedbus was left alone — the audit sink + typed hook cover creation with less coupling.Dashboard
GET /api/lifecycle-timelinenow serves{journeys, fleet}; the ACMM level filter gates whole journeys (agent-less journeys always pass).ref, stage chips along the fixed axis (enum → class → kick → pr → merged/blocked) with the current stage colored via the existingv4KindClassmapping and earlier stages dimmed, latest agent, last activity. Counters keep their markup and are now honest, with a coverage label derived fromfleet.coveredMs(“over the last 2h of recorded history (window 6h)”).EnableLifecyclePersistencemirrors theEnableSessionPersistencewiring on the PVC.Compatibility
pkg/retrokeeps working:applyTimelinereads kick cardinality from the journey's kicked-stageCountand PR refs/state from stage attrs — its existingTestReconstructexpectations (2 kicks, drift pause, merged PR ref) pass unchanged.annotatorAdapter(KindBlockedannotations) records into the same store unchanged;Recent/ByIssueremain as synthesized per-stage event views.Tests
-race.runEscalationSweeppass at threshold).gofmtclean on touched files;go vetclean; targeted suites green (pkg/timelineincl.-race,pkg/retro,pkg/scheduler,pkg/tracing, fullpkg/dashboard, touchedcmd/hivetests).🤖 Generated with Claude Code