timeline: includeManaged source parity + delta/deep-link contract tests#1147
Merged
Conversation
The retained endpoint has no include_managed param and the local path's 'all' preset re-includes managed rows server-side, so a consumer's includeManaged=false filtered in one mode and not the other — the TimelineSource abstraction wasn't substitutable. Enforce it client-side in applyClientFilters for both sources, mirroring the Go store's IsManaged predicate. Default (unset) keeps machinery rows, which the swimlane's pod/RS child lanes require — Radar's own UI is unchanged.
Three contracts whose regressions are invisible until a user hits them: - A store-agnostic conformance suite run against both EventStore implementations pins the Seq contract (strictly increasing arrival numbers, oldest-first delta pages that resume a burst beyond the page limit losslessly, relist dupes never re-delivered, K8s-event bumps re-arriving at the frontier exactly once). The two stores enforce it with unrelated mechanisms; a violation surfaces as silent client-side event loss, not an error. - The delta-sync client orchestration (full fetch → delta poll → epoch-mismatch resync) is extracted from the useChanges queryFn into runDeltaSyncFetch with state passed explicitly, and tested at the fetch boundary: cursor priming from headers, merge-by-id, empty-delta cursor advance, cached-reference return, epoch-reset resync, and the full+deltas ≡ fresh-full equivalence. - TimelineView's URL-param functions (parse/write/push-vs-replace/ equality) are exported as test seams and round-trip tested, pinning the deep-link contract and the back-forward loop fix.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a68d7f7. Configure here.
… suite The contract-level tests existed three times — mirrored per-store copies plus the new suite. The suite now lives once in pkg/timeline/storetest (httptest-style, importable by any EventStore implementation), MemoryStore runs it from the pkg module's own tests, SQLiteStore from internal, and the thirteen mirrored per-store copies are deleted. It also absorbs two properties the first cut missed: the cursor keys on arrival order (a late event with an older timestamp still lands past it), and enrichment preservation across bare/enriched bumps. Store files keep only implementation-specific behavior: migrations, quarantine, retention, timestamp encoding, ring recency.
The retained fetch key doesn't carry the flag, so flipping it alone served the previous filtered array. The local memo gets it too — it is client-enforced now and must not depend on riding the server query key.
Consolidation review recovered assertions the extraction dropped and gaps the first cut never had: the arrival-order property now exercises AppendBatch's per-row seq assignment and pins newest-first no-cursor ordering; a relist dupe must keep the ORIGINAL row untouched (keep-first event type), not merely stay collapsed; enrichment preservation is asserted through Query — the path consumers read — as well as GetEvent; and the package doc states the suite's honest scope (arrival-order/ delta/upsert identity, not a full EventStore certification).
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.

Follow-ups from the #1111 review that were deferred at merge, now done.
includeManaged parity (TimelineSource substitutability)
A consumer passing
includeManaged: falsegot server-side filtering in local mode but nothing in retained mode — the retained endpoint has noinclude_managedparam, and the local path's'all'preset re-includes managed rows anyway, so the flag's behavior depended on the source.applyClientFiltersnow enforces it uniformly for both sources, mirroring the Go store'sIsManagedpredicate (owned, or ReplicaSet/Pod/Event). Default (unset) keeps machinery rows — the swimlane's pod/RS child lanes need them — so Radar's own UI behavior is unchanged, and no Radar Hub change is needed.Contract tests
internal/timeline/store_conformance_test.go): one suite run against bothEventStoreimplementations, pinning the Seq/delta contract — strictly increasing arrival numbers, oldest-first delta pages that resume a burst beyond the page limit losslessly, relist dupes never re-delivered, K8s-event count bumps re-arriving at the frontier exactly once. The two stores enforce this with unrelated mechanisms; a violation is silent client-side event loss.client.deltaSync.test.ts): theuseChangesqueryFn body is extracted verbatim intorunDeltaSyncFetch(state passed explicitly; behavior identical) and tested at the fetch boundary — cursor priming from headers, merge-by-id, empty-but-headered delta advancing the cursor, cached-reference return on empty delta, epoch-reset full resync, and the full+deltas ≡ fresh-full equivalence.TimelineView.urlparams.test.ts):parseTimeMode/writeTimelineParams/onlyHighFreqDiffer/timeModeEqualexported as test seams and round-trip tested — includingmaxRangeDayscapping of hand-typed?from&toand the replace-vs-push rule that keeps window pans from flooding history (pinning the back/forward-loop fix).Verification
make tscclean · web vitest 69/69 (31 new) · k8s-ui vitest 1152/1152includeManaged: falsepath (which nothing in Radar's UI sets today)Note
Low Risk
Primary product change is client-side
includeManaged: falseparity, which the PR states Radar UI does not use today; the rest is test refactors and exports with behavior-preserving extraction of delta sync.Overview
includeManaged: falsenow behaves the same for local and retained timeline sources. Client-side filtering inapplyClientFiltersmirrors GoTimelineEvent.IsManaged(owned resources plus ReplicaSet/Pod/Event kinds). Unset/default still keeps machinery rows for swimlanes; only an explicitfalsestrips them.EventStore seq/delta behavior is centralized in
pkg/timeline/storetest. Duplicate informer-ID, K8s bump upsert, stale bump, enrichment preservation, and delta paging tests are removed from per-store SQLite/memory test files and run once viaRunConformancefor bothMemoryStoreandSQLiteStore.useChangesdelta orchestration is testable without React. The queryFn logic moves into exportedrunDeltaSyncFetchwith explicit meta/cache/now;client.deltaSync.test.tscovers cursor priming from headers, merge-by-id, empty deltas advancing the cursor, cached reference on no-op deltas, epoch-mismatch full resync, and incremental vs fresh-full equivalence.Timeline deep-link URL helpers are exported and covered (
parseTimeMode,writeTimelineParams,onlyHighFreqDiffer,timeModeEqual) inTimelineView.urlparams.test.tsfor round-trips, retention caps, and replace-vs-push history rules.Reviewed by Cursor Bugbot for commit b714938. Bugbot is set up for automated code reviews on this repo. Configure here.