Skip to content

node analytics coverage: follow-ups to #494 (late_evicted_observations semantics, fast-path separator test) #504

Closed

Description

@dborup

Follow-ups from the round-2 review of #494 (see "Review — CS-pve-agent3 PR#494" on head ef687ff0) and from the staging check.

  1. coverage.late_evicted_observations is misleading. It counts every observation after the evicted tx's FirstSeen + 1 ns, so normal second-observer hearings a few seconds later count too. On staging it rose by about 11,000 per hour (13,919 → 24,999 in 50 minutes). Count only observations that are genuinely late, for example more than 1 h after FirstSeen, and document the threshold. Alternatively, rename the field to say what it counts. No UI uses it today.
  2. The fast-path separator is not pinned. Mutant M7 (dropping the raw[13] != ':' check in parseCanonicalUTCSeconds) survives: 2026-10-16T12X30:00Z is accepted, while time.Parse rejects it. Add a parity test that covers every separator position.
  3. Nit. Revert the unrelated gofmt re-alignment in types.go and store.go, or leave it, but do not mix it with logic changes.

The staging check of the main fix passed: coverage.from tracked eviction smoothly over an hour, with a 48 h window and no jumps across 115 eviction ticks.

Activity

  1. dborup commented on Oct 20, 2026

    @dborup
    OwnerAuthor

    Fixed by #513 (merged as f56ef97c33d5b347bf1c2aba533d0d361d4f64dc).Fixed by #513: late_evicted_observations counts only genuinely late observations, and the fast-path separator parity test is added (APPROVE with nits, CI green).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions