ottyl/ideas/0003-timeline-ordering-invariant.md

# Idea: timeline ordering — confirm/harden the sequence-is-primary invariant

## Original note

"The timeline is time ordered. That is always derived from the
sequence ordering. That means the primary sequence is simply sequence
ordered." — reads as a statement of intent/invariant rather than a bug
report, so it's unclear whether this needs a code change or is just
confirming existing behavior.

## Current state (confirmed in code)

- `IndexKey::IngestedLog { sequence, timestamp }`
  (`src/engine/index_key.rs:10-13`) derives `Ord` with `sequence` as
  the primary field and `timestamp` only as a (practically unused)
  tie-breaker, since `sequence` is already unique.
- `sequence` is a strictly monotonic counter incremented per ingested
  line (`src/engine/ingest.rs:13,52`); `timestamp` is a separate
  wall-clock `Timestamp::now()` read at the same point in the ingest
  loop (`src/engine/ingest.rs:18,93`) — not derived from `sequence`.
- The default log listing path (`logs_in_sequence_range`,
  `src/engine/snapshot/mod.rs:180-208`, used by the `Logs` query in
  `src/engine/snapshot/query.rs:118-172`) is explicitly sequence-order,
  with an existing comment documenting this
  (`src/engine/snapshot/mod.rs:176-179`).
- The only other ordering mode is `sort_by` (sort by a declared param's
  value, sequence as tie-break, via `SortHelperKey`,
  `src/engine/snapshot/sort.rs`) — there's no "sort by timestamp" mode
  distinct from sequence order.
- **The one place ordering is genuinely timestamp-driven**: the
  `Timeline` query (`src/engine/snapshot/query.rs:173-198`) buckets
  logs by `timestamp.as_millis() / bucket_ms`
  (`:191`), not by sequence.

## Why this is an idea, not a ticket

The invariant the note describes already appears to hold for the
default log listing — sequence is the primary and effectively only
ordering key there. The one place `timestamp` (not `sequence`) drives
grouping is the `Timeline` bucketing, and `sequence`/`timestamp` are
captured independently rather than one being derived from the other.
In the current single-threaded ingest loop they're expected to
co-vary, but nothing guards against a backward wall-clock step (NTP
correction, clock skew) between two `Timestamp::now()` calls — if that
ever happened, sequence-ordered views and timestamp-bucketed views
could disagree on relative order for the affected lines. This has not
been observed as an actual bug; it's a latent gap worth a decision.

## Open question

Is the ask just "confirm this holds" (in which case: it does, for the
default listing — no code change needed), or do you want the
`Timeline` bucketing hardened so it can never diverge from sequence
order even under clock skew (e.g. clamp each line's timestamp to be
`>= previous line's timestamp`, or derive bucket assignment from
sequence position rather than raw wall-clock read)? The fix is small
either way, but scoping it as a ticket needs that decision first.

## Implications and things to consider

### Performance

- Clamping `timestamp` to be monotonic (`max(this_line_ts,
  previous_line_ts)`) is O(1) per line — negligible cost, just one
  extra comparison in the existing ingest loop
  (`src/engine/ingest.rs:13-18,52,93`). Deriving bucket assignment from
  sequence instead of wall-clock would be a larger change (see below)
  but isn't inherently more expensive at ingest time.
- No performance concern either way — this is a correctness/consistency
  question, not a hot-path cost question.

### Code structure

- If bucket assignment moves from wall-clock `timestamp` to something
  sequence-derived, `Timeline`'s bucketing
  (`src/engine/snapshot/query.rs:173-198`) still fundamentally needs a
  *time value* per bucket (users read timeline buckets as "logs between
  10:32:00 and 10:32:05", not "logs 100-150") — sequence can't replace
  timestamp as the bucketing axis, only guard it. The realistic fix is
  the monotonic-clamp approach, not a switch to sequence-based
  bucketing.
- Clamping at ingest time (`ingest.rs`) is the natural single place to
  fix this, rather than patching every read path that consumes
  `timestamp` — confirm there's exactly one ingest entry point today
  (there appear to be two: stdin/file tailing and Prometheus metrics
  polling, per `src/engine/ingest.rs` — a clock-skew guard needs to
  apply to both, and they'd need to share one "last observed timestamp"
  reference point to clamp against consistently).

### Functionality

- A clamped timestamp is no longer "true" wall-clock time for that
  line — if any UI surface displays a log's literal ingest timestamp
  (as opposed to just using it for bucketing), clamping would make a
  displayed timestamp lie slightly during a clock step-back event. Low
  probability, but worth deciding whether clamping is silent or
  surfaced (e.g. a flag on the log entry noting its timestamp was
  adjusted).
- This only matters at all during an actual backward clock step, which
  in most deployments (a piped CLI process, not a long-running server
  through NTP corrections) may never occur in practice — worth weighing
  whether this is worth any code change versus documenting it as an
  accepted limitation.

### Not yet considered

- No investigation into whether the same two independently-captured
  timestamp/sequence pattern exists anywhere else besides ingest (e.g.
  alert evaluation timestamps, or the Prometheus-polling ingest path
  specifically) — the research so far only confirmed the stdin/file
  ingest loop.
- No check on how a forward clock jump (not just backward) would
  interact with `Timeline` bucketing — a large forward jump would
  create a sparse/empty bucket range, which is a different (probably
  more common) failure mode than the backward-step case this idea
  focuses on, and might be worth addressing in the same pass if one
  happens.