Skip to content

anomaly: backlog signal for late-delivered observations - #591

Merged
dborup merged 5 commits into
masterfrom
feat/538-backlog-late-delivery-signal
Oct 25, 2026
Merged

dborup merged 5 commits into
masterfrom
feat/538-backlog-late-delivery-signal

Conversation

@dborup

@dborup dborup commented Oct 25, 2026 •

Copy link
Copy Markdown
Owner

Relates to #538

Summary

This adds an opt-in late-delivery flag to corescope-anomaly export, for cmd/anomaly only. It marks the events of an observer burst that delivers transmissions the rest of the network reported minutes earlier. That is the shape of a queue sent on reconnect and stamped at delivery.

The #473 flag ("first observations within 2 h after a 10-min silence of the observer") marked about 75 % of the events in two of three real GRP_TXT storms (#473 issuecomment-6380316698). The new flag needs no silence, and the synthetic real storms in the tests do not trigger it.

  • It writes the existing "ingest_backlog": true, so the rate rule's exclude_ingest_backlog guard reads it unchanged. internal/anomaly, replay and the replay goldens are not touched (git diff origin/master -- internal/ is empty).
  • Four explicit flags: -late-window, -late-min-rows, -late-share and -late-gap. There are no defaults. The four must be given together, and they cannot be combined with -backlog-*. Without them the output is byte-identical to master.
  • Key-free: the export reads observations.observer_idx into memory only, a line gains the boolean and nothing else, and the output stays byte-identical when observers are renamed.
  • No ingestor, server, API or UI wiring. No production thresholds or defaults.

Plan (this PR)

  1. Evaluate the three candidates from anomaly: design a backlog signal that marks late-delivered observations (follow-up to #473) #538 on the e2e fixture and on synthetic data. No staging data is read.
  2. Implement the chosen signal in the export, tests first. Cover the definition at every bound, privacy (as in Anomaly round 2: rate rule with backlog guard #477/anomaly round 2: rate rule with an ingest-backlog guard, blind validation on fresh data #473), opt-in, bounded cost, and the CLI.
  3. Add an end-to-end replay test (unguarded rule, the new guard, and the anomaly round 2: rate rule with an ingest-backlog guard, blind validation on fresh data #473 guard on the same data) and a randomised 20-seed sweep through the real export.
  4. Draft a pre-registration for a validation round (below). It is not posted; the coordinator posts it.

Candidates evaluated

Candidate Finding Decision
Arrival lag per observation (on-air time vs ingest time) The database has no ingest-time column. observations.timestamp is the envelope receive time, or the ingest time where resolveRxTime falls back to it. The only proxy is row-id order: "lag = max timestamp of earlier-inserted rows − own timestamp". On the e2e fixture that proxy puts 487 of 500 rows over 5 min, because the fixture's ids are not in ingest order [A]. A copied or rebuilt database breaks the proxy silently. It is also blind to the case that matters: a queue stamped at delivery has lag ≈ 0, and the #457 artifact definition (a storm whose transmissions were first heard by one observer that was silent before it) implies timestamps bunched at the reconnect. A queue that keeps its receive times does not form a timestamp-space storm at all. Rejected
Observer burst shape (dense vs the observer's own rate) A real storm is also a dense burst for every observer that hears it. On its own this is close to what the #473 flag measured. Kept only as the -late-min-rows condition
"Already seen" share A delivered queue repeats what live observers heard minutes before. A live observer hears the same transmissions within seconds of its peers, or is the only one to hear them. This separates the two cases without a silence condition. Chosen, with two refinements (below)

Definition

  • Already seen. For a row of observer O at time t, e is the latest observation of the same transmission by another observer before t + gap, over the whole database. The row is already seen when such an e exists and d = t − e ≥ gap. That means others heard it at least gap before, and none of them within gap of it.
    • Refinement 1: "latest other, before t + gap" rather than "earliest other". Otherwise one observer whose clock runs behind would make every live peer look late. The test scenario and mutant M6 cover this.
  • Late burst. Bins are epoch-aligned and window wide. O's bin k is a late burst when O's rows in bins k−1, k and k+1 meet all three conditions:
    • they number at least min-rows;
    • at least share of them are already seen;
    • the delays of the already-seen rows span at least gap.
    • Refinement 2: the spread condition. A queue holds transmissions heard over the whole outage, so its delays spread from about 0 to the outage length. A skewed clock gives a constant delay. Mutant M2 covers this.
  • Flag. An event is flagged when its first observation (the one that sets its time) is a row of O in a late burst of O. Rows with a NULL observer are never flagged.

Evaluation (synthetic data and e2e fixture only; no staging data)

Scenario fixture (TestExportLateDeliveryFlagsTheDrainBacklogOnly): window 1m, min rows 10, share 0.5, gap 2m. The #473 flag uses 10m/2h [T].

Part Events #473 flag Late-delivery flag
background ADVERTs (A, B, Drain live) 120 26 0
drain backlog (40-min outage, queue delivered at once) 25 25 25
real storm, first heard by an observer silent for hours (the #473 loss) 30 30 0
real storm, one observer's clock 161 s behind 40 30 0
real storm, one isolated observer hears it alone 25 25 0
first observation without an observer inside the drain burst 1 0 0

End to end (TestExportThenGuardedReplayIgnoresTheLateBacklog), with a GRP_TXT rate rule (threshold 20 / 5 min) [T]:

Randomised sweep (TestLateDeliverySyntheticSweep, 20 seeds, through the real export) [T]. Each seed has:

  • 8 observers, two of them with clocks 3–10 min behind (different offsets);
  • ADVERT background every 30 s;
  • two real storms of 30–150 transmissions over 20 s–5 min, with a random hearing probability of 0.1–0.9;
  • one outage of 5–40 min, after which that observer delivers its queue (its outage rows plus 10–40 transmissions it alone heard), stamped at delivery.

A "near" event is a live event first heard by the draining observer within two windows of its delivery.

Share Live events flagged Near events flagged Delivered-queue events flagged
0.5 (proposed) 0 / 8769 1 / 11 229 / 415 (55.2 %)
0.3 1 / 8769 7 / 11 320 / 415 (77.1 %)
0.25 1 / 8769 7 / 11 375 / 415 (90.4 %)
0.2 13 / 8769 7 / 11 375 / 415 (90.4 %)
#473 flag (10m/2h) 249 / 8769 (2.8 %) 11 / 11 415 / 415
  • At share 0.5 the flag marks no live event on any seed, against 2.8 % for the anomaly round 2: rate rule with an ingest-backlog guard, blind validation on fresh data #473 flag.
  • The single live event flagged at 0.3 involves two skewed clocks with different offsets. When only those two heard a transmission besides O, O's row looks late with two different delays, so the spread condition passes.
  • Recall depends on the queue's mix. A queue with fewer already-seen rows than share of the burst, for example a short outage with many transmissions only the draining observer heard, is not flagged. The sweep mixes small queues (10–40) with sparse background (about one shared row per minute of outage). A busier network raises recall only after long outages: see the large-queue limit below. 55 % is only a synthetic figure.

e2e fixture (TestExportLateDeliveryE2EFixture): over the whole fixture day the late-delivery flag marks 0 of 498 events and finds 0 rows already seen. That is expected, because the fixture holds one observation per transmission, and it shows only that real single-observer traffic is not flagged. For contrast, the #473 flag (10m/30m, the only lookback that fits the fixture) marks 140 of 222 events in 17:00–18:00 of ordinary traffic [A] [T].

Limitations (stated before any validation data)

The first three are pinned in late_limits_538_test.go at window 1m, min rows 10, share 0.5 and gap 2m [T].

  • A common-cause outage is never flagged. When two or more observers lose the same uplink or broker and drain the same outage together, each queued row has a peer, the other draining observer, seconds away. That peer is the latest other observation before t + gap, so d ≈ 0, no row is already seen and nothing is flagged. This is the flip side of refinement 1 ("latest", not "earliest"). Synthetic, FIFO queue delivered at 1 row/s (TestLateDeliveryMissesACommonCauseOutage):

    Background Outage Queue-only GRP_TXT 1 observer drains 2 drain together 3 drain together
    every 60 s 40 min 25 25/25 0/25 0/25
    every 10 s 20 min 40 40/40 0/40 0/40
    every 10 s 40 min 40 40/40 0/40 0/40
  • Recall is low for a large queue after a short outage. The share's denominator holds the queue-only rows and the observer's own live rows after reconnect. A queue large enough to trip the GRP_TXT rule on its own (≥ ~33 queue-only rows) is therefore flagged only when at least as many already-seen rows, older than the gap, come with it in the same 3-min window. 40 queue-only GRP_TXT, FIFO, delivered at 5 rows/s (TestLateDeliveryRecallOfALargeQueue):

    Shared rows per minute Outage 3, 5, 10 min Outage 20, 40 min
    6 0/40 40/40
    1 0/40 0/40
  • Share 0.5 is no protection against skewed clocks. Two observers whose clocks run behind by different offsets of at least gap give another observer's rows two different delays, which can pass the spread condition at any share. A real storm of 40 GRP_TXT heard by O (correct clock), a third of it also by an observer 3 min behind and another third by one 7 min behind, and by no one else: 13 of 40 events are flagged at 1m/10/0.5/2m (TestLateDeliveryTwoSkewedClocksDefeatTheShare). In the sweep, 0.5 happened to flag no live event and 0.3 one; that is a property of the sweep's networks, not a guarantee.

  • An outage shorter than about 2·gap (4 min) is never flagged: already-seen delays are at least gap, and their spread must be at least gap, so the oldest queued row must be at least 2·gap old [A].

  • A queue from an observer whose coverage overlaps no other observer has no already-seen rows, so it is not flagged.

  • A queue that keeps its receive times is not flagged. It forms no timestamp-space storm either.

  • An observer's own live first observations within one window of its delivery are flagged too, because the flag marks whole bursts.

  • The share bound is compared in floating point (seen >= share * rows). It is exact for shares that are multiples of 1/2^k, such as 0.5, but misses some decimal bounds: at share 0.07, 7 of 100 rows seen is not a burst, because 0.07 · 100 = 7.000000000000001 [T]. Documented, not changed.

  • gap must exceed propagation plus upload jitter. 2 min is 4× the 30 s settle window that the export already treats as "live evidence".

Privacy

As in #477/#473:

  • lateQuery reads only observer_idx, timestamp, id and transmission_id, with no join (TestLateQueryReadsOnlyActivity).
  • With the flag stripped, the export is byte-identical to the plain one, and the only new key is ingest_backlog, whose only value is true (TestExportLateDeliveryAddsOnlyTheBoolean).
  • Renaming observers (three bijections, one of them parity-changing) leaves the export and the summary byte-identical (TestExportLateDeliveryIsInvariantUnderObserverRenaming).
  • The summary late_delivery holds counts only: rows, rows without an observer, rows already seen and events flagged.

Cost

This is an offline tool, not a server hot path. The flag adds one index range scan on observations.timestamp over [start − settle − 2·window, end + settle + 2·window), under the same -max-rows cap, with one index search per row over its transmission's observations. TestLateQueryPlanIsBounded asserts no SCAN and no TEMP B-TREE. Memory is one entry per (observer, bin) with rows, capped at 4,194,304; one more fails the export (TestLateTrackerIsBounded). No performance claim is made.

Tests and mutants

Red before: on origin/master the new tests do not build, because exportOptions has no Late* fields and exportSummary has no LateDelivery [T]. Each requirement also has assertion-level mutants. A mutant is applied to the committed source, the cmd/anomaly tests run, and the file is restored with git checkout [T].

Requirement Tests (all in cmd/anomaly) Mutants killed
Marks late-delivered observations, not first observations after a silence TestExportLateDeliveryFlagsTheDrainBacklogOnly, TestLateTrackerBurst, TestLateTrackerBinsAndSeen, TestExportThenGuardedReplayIgnoresTheLateBacklog, TestLateDeliverySyntheticSweep, TestExportLateDeliverySummaryCountsTheScan M1 flag never set · M2 no spread condition · M3 already seen needs d > gap · M4 strict share · M5 window = own bin only · M6 earliest instead of latest other observer · M7 no t + gap bound · M8 own rows count as "another observer" · M18 NULL observer flagged
Key-free: only a boolean per event; byte-identical output under observer renaming (#477-style) TestExportLateDeliveryAddsOnlyTheBoolean, TestExportLateDeliveryIsInvariantUnderObserverRenaming, TestLateQueryReadsOnlyActivity M9 flag depends on observer identity (parity) · M10 activity scan reads snr
Opt-in, no defaults, exclusive with -backlog-*; plain export and replay goldens unchanged TestExportLateDeliveryIsOptIn, TestExportCommandLateDeliveryFlags, all existing export, replay and golden tests (unchanged and green) M11 late() ignores -late-share · M12 no exclusion with -backlog-* · M13 share bound 2 · M14 a default -late-window (kills 6 existing command tests)
Bounded cost TestLateQueryPlanIsBounded, TestLateTrackerIsBounded, TestExportLateDeliveryScanIsCappedByMaxRows, TestExportLateDeliverySummaryCountsTheScan M15 bin cap off by one · M16 -max-rows off by one · M17 scan margin without the 2 windows
Real data before 2026-10-25 TestExportLateDeliveryE2EFixture (e2e fixture, unchanged file hash) covered by M1/M10
Stated limits and the exclusive t + gap bound (review round 2) TestLateDeliveryMissesACommonCauseOutage, TestLateDeliveryRecallOfALargeQueue, TestLateDeliveryTwoSkewedClocksDefeatTheShare, TestLateQueryPeerAtExactlyTheGapIsNotWithin MF3 peers within the gap ignored (p.timestamp <= o.timestamp − gap) · MF4 queue-only rows leave the share's denominator · MF5 spread must exceed 2·gap · MF6 t + gap bound inclusive (the reviewer's X1)
  • M19 ("the plain export reads the observer column when late()") was equivalent and is dropped: late() implies the flag is set.
  • internal/anomaly, cmd/server, cmd/ingestor, public/ and .github/ are unchanged.

DRAFT pre-registration for a validation round (not posted; the coordinator posts it)

DRAFT. This is not posted, and no round exists until the coordinator posts it, filling in the one placeholder <HEAD SHA> (defined below) before 2026-11-02T00:00Z and before any data from V is seen. The author has no staging access and has seen no staging data. Everything in it was derived from #457/#473 published aggregates, the e2e fixture and synthetic data.

Rules of the round

  • Everything below is fixed once posted. Changing a flag value, a rule value, the labelling, a metric or a bar after the data is seen starts a new round, reported separately; it is never a pass of this round.
  • The comment is not edited after posting. A correction is a new comment and starts a new round.

Tool. cmd/anomaly from this PR at head <HEAD SHA>, built with go build -trimpath; the binary sha256 is reported. <HEAD SHA> is the head commit of PR #591 that the last review of the PR reviewed: a commit on feat/538-backlog-late-delivery-signal, not a merge commit and not master. If that commit cannot be built, the round is void.

1. Data and runs

Item Fixed value
Validation window V [2026-11-02T00:00Z, 2026-11-09T00:00Z): 7 whole UTC days. V is disjoint from #474 round 4, whose V ends at the latest on 2026-11-01T00:00Z, and starts one day after it, so the export's scan margin (V.start − 30 s − 2 min) reads no row of that round's V either. The snapshot must cover V plus the settle margin plus the scan margin of 2 windows: it is taken after V.end + 30 s + 2 min. There is no shorter fallback. If this text is not posted before 2026-11-02T00:00Z, or the coordinator has seen data from V before posting, the round is void; a new round needs a new posting with a new V.
Calibration day C [2026-11-02T00:00Z, 2026-11-03T00:00Z), the first 24 h of V. It is used only for the rate thresholds (§3).
Evaluation window E [2026-11-03T00:00Z, 2026-11-09T00:00Z), V minus C, 6 days. All metrics count only episodes activated in E and storms that start in E.
Snapshot As in #473 §1: an offline scripts/backup-sqlite.sh copy used as-is when no -wal/-journal file sits beside it, or else sqlite3 -readonly <source> "VACUUM INTO …". Read-only, with -immutable.
Export (one file, both arms) corescope-anomaly export -db <snap>/meshcore.db -immutable -start 2026-11-02T00:00:00Z -end 2026-11-09T00:00:00Z -settle 30s -max-rows 50000000 -late-window 1m -late-min-rows 10 -late-share 0.5 -late-gap 2m -out events.jsonl
Replay, arm A / arm B corescope-anomaly replay -config round538A.json (or round538B.json) -input events.jsonl -start 2026-11-02T00:00:00Z -end 2026-11-09T00:00:00Z -max-line-bytes 4096 -out reportA.json (or reportB.json)
Episode One EventID. Activation is its first transition to active; its end is the last transition to ended (V.end if none).
Capacity #457 round 2 §1a, plus a bounded retry. The export runs with -max-rows 50000000. If either export scan fails with more than -max-rows, it runs once more with -max-rows 100000000, the tool's maximum (maxExportRows); no other value is used. If that run also fails with more than -max-rows, the round is void. A failure for more than 4,194,304 (observer, bin) entries also voids the round. Every run and its outcome is reported.

2. The late-delivery flag (fixed values)

The definition is the one in this PR:

Why these values. All of them come from data before V, and none from staging after 2026-10-24:

  • gap = 2 min is 4× the 30 s settle that the export already treats as live route evidence. Flood propagation and upload jitter sit well inside it.
  • window = 1 min makes the 3-min window hold a delivery burst of seconds, plus its neighbours across a bin edge.
  • min rows = 10 is under a third of the round-2 GRP_TXT threshold (33 per 5 min, anomaly round 2: rate rule with an ingest-backlog guard, blind validation on fresh data #473 issuecomment-6380305586). A burst too small to matter to the rate rule on its own is still flagged when it is clearly late.
  • share = 0.5 is the largest value tested. In the 20-seed synthetic sweep it is the only one that flagged no live event; 0.3 flagged 1 of 8,769, a case with two skewed clocks. That is a property of the sweep, not a protection: two clocks behind by different offsets can make another observer's rows late at any share (limitations below).

Limitations, stated before the data (synthetic figures from this PR's tests):

  • A common-cause outage is never flagged. When two or more observers drain the same outage together (a shared uplink or broker), each queued row has the other draining observer as a peer within seconds, so no row is already seen. One observer draining a 40-min outage: 25/25 and 40/40 flagged; two or three together: 0. Episodes that such outages cause stay in arm B as in arm A.
  • A queue whose already-seen rows are fewer than half of the burst is not flagged. The sweep's recall was 55 %. For a queue of 40 queue-only GRP_TXT delivered at 5 rows/s, recall is 0/40 after outages of 3–10 min at 6 shared rows/min and after 3–40 min at 1 shared row/min, and 40/40 after 20 and 40 min at 6 shared rows/min.
  • An outage shorter than about 4 min (2·gap) is never flagged.
  • A queue from an observer that overlaps no other observer is not flagged.
  • The draining observer's own live first observations within a window of its delivery are flagged.
  • Share 0.5 does not protect against skewed clocks. Two or more clocks behind by different offsets of at least 2 min can make another observer's rows look late: in a synthetic real storm of 40 GRP_TXT heard by O and, a third each, by observers 3 and 7 min behind, 13 of 40 events were flagged. Such a storm can lose its episode in arm B; the comparative criterion (§6) counts that as a loss.

3. Rule configs

4. Ground truth

The #457 round-2 §4 rate labels, unchanged, computed with label457.py. Only V and E change, to the values in §1, and only the rate part is used.

  • Storm bins are count ≥ max(3·B(p,h), B(p,h)+30), and runs separated by one bin are merged.
  • Ingest artifact: ≥ 50 % of a storm's transmissions were first observed by one observer that had no observation in the 10 min before the storm.
  • Counts use all events, flagged ones included. The labels use no detector output and not the flag.
  • The label file holds (payload type, start, stop, artifact) tuples only. Its sha256 is posted before either replay runs.

5. Matching and metrics

Per arm, as in #457 round 2 §5 (rate column), unchanged. The recount for arm B counts only lines without "ingest_backlog": true.

Comparative metrics (they decide, §6). An episode is on a storm, and a storm is detected by an arm, under the #457 round 2 §5 matching rule, unchanged. Over the storms that start in E:

  • N_art and N_real: the number of artifact and non-artifact storms;
  • EA_art and EB_art: the number of arm A's and arm B's episodes that are on an artifact storm;
  • L: the number of non-artifact storms that arm A detects and arm B does not.

Also reported, outside the bars:

  • per-rule numbers;
  • artifact storms, and each arm's episodes on them;
  • late_delivery.events_flagged and rows_already_seen, in V and in E;
  • per storm, the share of its events that are flagged;
  • the episodes of A without an overlapping episode in B, and the reverse, as counts and tuples;
  • the non-artifact storms that arm B detects and arm A does not.

6. Pass bars and decision

All of these must hold, otherwise no-go:

  • recount ≥ threshold at 100 % of activations;
  • precision ≥ 0.70;
  • recall ≥ 0.80;
  • false alarms ≤ 1.0/day;
  • median TTD ≤ 10 min;
  • median stop latency ≤ 20 min.

Comparative criterion, also required for a go (§5 metrics; the comparator is arm A, the tolerance is zero storms):

  • C1, benefit: N_art ≥ 1 and EB_art < EA_art. Arm B has strictly fewer episodes on artifact storms than arm A.
  • C2, no loss: L = 0. Arm B detects every non-artifact storm that arm A detects. This implies that arm B's recall is at least arm A's.

How the result is read:

  • Go = arm B passes every bar above and C1 and C2. Arm A's own bars do not decide; arm A is the comparator for C1 and C2.
  • What a go licenses: on this network in E, with the fixed flag values of §2 and the two round-1 rate rules of §3, adding the late-delivery guard reduced episodes on anomaly phase 3: replay CLI, read-only export and blind validation (periodic, rate, new-stream) #457-labelled ingest artifacts compared with the same rules unguarded, lost no non-artifact storm that the unguarded rules detected, and met the anomaly phase 3: replay CLI, read-only export and blind validation (periodic, rate, new-stream) #457 bars. It licenses nothing else: no default or production value; no claim about other rules, windows, flag values or networks; no claim that the flag finds every backlog (recall is not measured against backlogs, only against artifact-storm episodes); and no claim about common-cause outages, which are never flagged (§2).
  • No-go = any bar fails, or C2 fails, or N_art ≥ 1 and EB_art ≥ EA_art.
  • Zero artifact storms in E (N_art = 0), or arm A with no episode on an artifact storm (EA_art = 0), leaves the guard's benefit unmeasured: the round is inconclusive, which counts as a no-go for any claim. C2 and the bars are still reported, as a safety result only.
  • Zero non-artifact storms in E makes the round inconclusive, which counts as a no-go for any claim.
  • A void round (§1: posting after V.start or after data from V was seen, an unbuildable <HEAD SHA>, a failed export at -max-rows 100000000, or the bin cap) has no results.
  • No value from this round becomes a default or a production value.

7. Files

events.jsonl, both reports and the label file stay with the coordinator. Only aggregates and (payload type, start, stop, artifact) tuples are published.

🤖 Generated with Claude Code

dborup and others added 3 commits October 25, 2026 09:42
Add an opt-in export flag that marks the events of an observer burst
delivering transmissions other observers reported minutes earlier: a
reconnect queue stamped at delivery. It sets the existing ingest_backlog
boolean, so the rate rule's exclude_ingest_backlog guard reads it
unchanged; the library, replay and goldens are untouched.

A row is already seen when another observer heard its transmission at
least -late-gap before it and none within the gap. An observer's bin is a
late burst when its window (the bin and both neighbours) holds at least
-late-min-rows rows, at least -late-share of them already seen, with
those delays spread over at least the gap. Unlike the #473 flag it needs
no silence, so a real storm that an often-quiet observer hears first is
not flagged.

Four explicit flags, no defaults, exclusive with -backlog-*. Only the
boolean leaves the export; the output is byte-identical under observer
renaming.

Relates to #538

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Relates to #538

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ivacy test

Relates to #538

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dborup

dborup commented Oct 25, 2026

Copy link
Copy Markdown
Owner Author

Rapport — CS-Minimax PR#591 #538 — head fca73b3

Status: Draft. All requirements are implemented and tested, and every CI job is green (no reruns). The draft pre-registration is in the PR description, not posted.

Evidence: [T] test or command output, [A] analysis or measurement, [K] code or config read.

Requirements

Requirement Result Tests Mutants (killed)
Better backlog signal: marks late-delivered observations, not first observations after a silence (cmd/anomaly export only, opt-in) Done. Late-delivery flag: an observer burst whose rows repeat transmissions other observers heard at least gap earlier and none within gap, with those delays spread over at least gap. It writes the existing ingest_backlog boolean. [K][T] TestExportLateDeliveryFlagsTheDrainBacklogOnly, TestLateTrackerBurst, TestLateTrackerBinsAndSeen, TestExportThenGuardedReplayIgnoresTheLateBacklog, TestExportLateDeliverySummaryCountsTheScan M1–M8, M18 [T]
Candidates evaluated on synthetic backlogs and real-looking storms that must not be flagged Arrival lag rejected: there is no ingest-time column, and the id-order proxy puts 487 of 500 e2e-fixture rows over 5 min [A]. Burst shape is kept as min-rows; the "already seen" share is chosen. Scenario fixture: backlog 25/25 flagged; quiet-observer, skewed-clock and isolated storms 0 flagged (the #473 flag: 30, 30, 25) [T]. 20-seed sweep at share 0.5: live events 0/8769 flagged (#473: 249), delivered-queue events 229/415 [T]. TestLateDeliverySyntheticSweep, TestExportLateDeliveryE2EFixture M2, M6, M8, M9 [T]
Only data before 2026-10-25; no staging or prod access Only synthetic data and the e2e fixture (2026-05-17) were used. No ssh, staging or prod access, and no API key used. [K] TestExportLateDeliveryE2EFixture (fixture hash unchanged) –
Key-free: boolean per event, no observer identity, #477-style privacy tests With the flag stripped, the output is byte-identical to the plain export, and the only new key is ingest_backlog: true. Three observer renamings (one parity-changing) leave the output and the summary byte-identical. The activity scan reads only observer_idx, timestamp, id and transmission_id. [T] TestExportLateDeliveryAddsOnlyTheBoolean, TestExportLateDeliveryIsInvariantUnderObserverRenaming, TestLateQueryReadsOnlyActivity M9, M10 [T]
Replay goldens unchanged without the option; no defaults internal/anomaly is untouched (git diff origin/master -- internal/ is empty) and its tests pass. The four flags have no defaults, must come together, and exclude -backlog-*. [T][K] TestExportLateDeliveryIsOptIn, TestExportCommandLateDeliveryFlags, existing golden and export tests M11–M14 [T]
Bounded cost One index range scan plus one index search per row (no SCAN, no TEMP B-TREE). Bins are capped at 4,194,304, and the scan is capped by -max-rows. [T] TestLateQueryPlanIsBounded, TestLateTrackerIsBounded, TestExportLateDeliveryScanIsCappedByMaxRows M15–M17 [T]
Design, justification and DRAFT pre-registration in the PR description Done. The draft is not posted, and the coordinator fills in <V.start>/<HEAD SHA>. [K] – –
  • Red before: on origin/master the new tests do not build (exportOptions has no Late* fields) [T].
  • M19 was an equivalent mutant and was dropped [A].

Invariants

Check Result
cmd/server read-only Unchanged by this PR [K]
New map[string]interface{} outside tests 0 added lines [T]
Hardcoded colours No frontend changes [K]
scripts/check-xss-sinks.sh --diff origin/master Clean: no public/ changes [T]
Fork guards deploy.yml 9 and release-fast-path.yml 1, both unchanged from master [T]

Local runs

Suite Result
cmd/anomaly go test ./... ok [T]
internal/anomaly ok [T]
cmd/ingestor ok [T]
cmd/server ok [T]
sh test-all.sh 271/271 [T]
node test-frontend-helpers.js 725/725 [T]
E2E No E2E touches this change. As a sanity run, test-e2e-playwright.js against a local Go server on a CI-prepared copy of e2e-fixture.db passed 154/157 with 3 skipped and 0 failed; the server was stopped by pid [T]

CI per job (head fca73b3)

Job Result
Go Build & Test pass
Go Ingestor Tests pass
Go Libraries & CLI pass
Go Server Tests 1/3, 2/3, 3/3 pass
JavaScript Unit Tests & Lint pass
Script & Container Unit Checks pass
Playwright E2E 1/3, 2/3, 3/3 pass
Verify E2E Shards & Coverage pass
Verify Go Test Assignments pass
Validate Docker Image pass
Build & Publish Docker Image pass
Publish Badges, Release Artifacts, Deploy Staging skipped (fork / PR)

No reruns were needed; #271 did not occur. [T]

Remaining

  • Synthetic recall. At share 0.5, recall on synthetic queues is 55 %. A queue with fewer already-seen rows than half the burst is not flagged. Lower shares raise recall but begin to flag live events when two clocks are skewed by different offsets: at 0.3, 1 of 8769 [T].
  • Unflagged queues. A queue from an observer with no overlap with others is not flagged, and neither is a queue that keeps its receive times [A].
  • Unvalidated on real backlogs. The e2e fixture holds one observation per transmission, so it cannot show real already-seen rows. Real-data behaviour is open until a validation round runs [A].
  • Not yet posted. The pre-registration needs the coordinator to post it and fill in the placeholders.

@dborup-agent

Copy link
Copy Markdown
Collaborator

Review — CS-pve-agent3 PR#591 — head fca73b3

Dom: REQUEST CHANGES. The code is sound: it is opt-in, key-free and byte-identical without the flags. The changes requested are to the DRAFT pre-registration (F1, F2) and to the stated limitations (F3, F5), plus one boundary test (F6). No functional code change is required.

This is an independent, read-only review. Evidence: [T] test or command I ran, [A] analysis, [K] code, config or issue read.

Findings

# Severity Where Finding
F1 Medium Pre-reg §5–§6 No bar measures the guard's benefit. Arm B is scored only against the absolute #457 bars, and in #473 round 2 the unguarded arm A already passed every bar. Arm B can therefore pass while the flag does nothing: with 0 events flagged, arm B is identical to arm A. events_flagged and "episodes of A without B" are only reported, outside the bars. Add a pre-registered comparative criterion. One option: if E holds at least one artifact storm, arm B has fewer episodes on artifact storms than arm A (or none) and loses no non-artifact storm that arm A detects. Also state what claim a "go" licenses. [K][A]
F2 Medium Pre-reg §1, §6 Judgement left after data. (a) <V.start> is "the first 00:00Z after posting at which the coordinator has seen no data". That is not a fixed rule, and it collides with #474 round 4, whose V starts at T0 = 2026-10-25T00:00Z and runs up to 7 days; the coordinator will see that data. Fix V.start as an explicit date in the posted text, and state whether V may overlap #474 round 4's V (preferably disjoint, on or after its V.end). (b) "Arm B can then pass only by not losing recall" has no comparator and no tolerance: relative to arm A? by zero storms? (c) The capacity rule ("-max-rows is doubled") is unbounded, but maxExportRows is 100,000,000, so 50M → 100M → refused. State the stop or void rule. (d) Say whether <HEAD SHA> is this PR's head or the merge commit. [K]
F3 Medium Design / limitations A common-cause outage is never flagged. When two or more observers drain the same outage together (a shared uplink or broker), each queue row has a peer (the other draining observer) within seconds. So "latest other before t + gap" is that peer, d ≈ 0, no row is already seen, and nothing is flagged. Reproduced in the scenario fixture's style. With a 40-min outage, 25 queue-only GRP_TXT and background every 60 s: one observer drains: 20/25 flagged; two drain together: 0/25; three: 0/25. With a FIFO queue, background every 10 s, 20- or 40-min outages and 40 queue-only GRP_TXT: one drains: 40/40; two together: 0/40. This is the flip side of refinement 1 ("latest", not "earliest"). It is in neither the PR's Limitations nor pre-reg §2. Add it before any data is seen, and ideally pin it in a test. [T][A]
F4 Low Recall Recall at storm sizes that matter to the rule. The share's denominator holds the queue-only rows and the observer's own live rows after reconnect. A queue large enough to trip the GRP_TXT rule on its own (≥ ~33 queue-only rows) therefore needs at least as many already-seen rows older than the gap in the same 3-min window. FIFO queue, 40 queue-only GRP_TXT, delivered at 5 rows/s: 0/40 flagged for outages of 3–10 min with 6 shared rows/min, and for 3–40 min with 1 shared row/min; 40/40 for outages of 20 and 40 min with 6 shared rows/min. The sweep's 55 % mixes in small queues (10–40), so calling it "pessimistic for a busier network" holds only for long outages. Suggest stating this in the PR and in pre-reg §2. [T]
F5 Low PR text, pre-reg §2 Two skewed clocks also defeat share 0.5. In a real storm of 40 GRP_TXT heard by O (correct clock) and by two observers whose clocks run 3 and 7 min behind, and by no one else, 13/40 of the storm's events are flagged at the PR's values (1m/10/0.5/2m). The limitation reads "the sweep shows this at share ≤ 0.3, not at 0.5", and §2 justifies 0.5 as "the only value that flagged no live event". That is true of the sweep, not in general. Reword it so that 0.5 is not presented as protection. [T]
F6 Low Tests The exclusive t + gap bound is untested. My mutant X1 (p.timestamp < o.timestamp + ?4 → <=) survives the full cmd/anomaly suite. Add a row whose only later peer sits exactly at t + gap. [T]
F7 Nit late.go flagged float64(w.seen) >= l.share*float64(w.rows) misses some exact bounds. At share 0.07 / 100 rows / 7 seen, 0.14 / 100 / 14 and 0.56 / 25 / 14, the burst is not flagged, because for example 0.07·100 = 7.000000000000001. It is exact for the pre-registered 0.5. Either document it or compare without the rounding. [T]
F8 Nit Limitations All already-seen delays are ≥ gap and their spread must be ≥ gap, so the oldest queued row must be ≥ 2·gap (4 min) old. An outage shorter than about 4 min can never be flagged. It is worth one line in the limitations. [A]

Answers to the review points

  • Marks late-delivered observations, not first observations after a silence. Yes, by construction [K]. The scenario fixture, the guarded replay and the 20-seed sweep pass on the merged tree [T]. My own reproductions confirm it: a single draining observer is flagged, and live traffic around it is not (0 of 120 and 0 of 720 background events in every scenario) [T]. F3 and F4 bound where it works.
  • Real storms not flagged. This holds for the quiet-first, isolated and single-skew storms (PR tests) [T]. It does not hold for a storm covered only by O plus two differently skewed clocks (F5) [T].
  • Key-free.
    • lateQuery reads observer_idx, timestamp, id and transmission_id only, with no join [K].
    • The observer is used only as a map key and in <> [K].
    • Beyond the PR's three bijections, I applied a random permutation of observer_idx onto unrelated large integers on sweep seeds 1–5. Export and summary stayed byte-identical (with 11, 16, 15, 0 and 13 flagged) [T].
    • The only new line key is ingest_backlog: true. The summary holds counts only [K][T].
  • Replay goldens unchanged without the option; no defaults.
    • git diff origin/master...head -- internal/ is empty. The only change outside the new files is the buildFixtureOf refactor in backlog_473_test.go [K].
    • All four flags default to 0. A partial set exits 2, and combining them with -backlog-* exits 2 [T].
    • CLI on the CI fixture copy, master binary vs merged binary:
      • the plain export is byte-identical (sha 95c26cd9…), and so is its summary;
      • the -backlog-* export is byte-identical (sha 6500b231…, 157 flagged), which covers the exportRecords refactor;
      • with the -late-* flags the output equals the plain export (0 already seen, 0 flagged);
      • an over-cap -max-rows fails without writing a file [T].
  • Bounded cost.
    • On the CI-migrated fixture (real dbschema indexes, including idx_observations_transmission_id), EXPLAIN QUERY PLAN still picks idx_observations_timestamp for the scan and idx_observations_tx_ts for the correlated subquery, with no SCAN and no TEMP B-TREE [T].
    • The bin cap and the -max-rows cap are enforced [K][T].
  • No staging data from 2026-10-25 onward.
    • The diff contains only synthetic fixtures (fxT = 2030-03-01) and the committed e2e fixture (2026-05-17) [K].
    • No new data files. test-fixtures/ is unchanged [K].
    • The cited numbers (threshold 33, the 75 % loss) are anomaly round 2: rate rule with an ingest-backlog guard, blind validation on fresh data #473's published aggregates from before 2026-10-25 [K].
    • I cannot verify the author's process beyond the artefacts.
    • I used no staging or prod data. My local E2E fixture contains CI's own synthetic "now" seed rows, not staging data.
  • DRAFT pre-registration: is everything fixed before data? Not yet. See F1 and F2. F3 and F5 also belong in its "Limitations, stated before the data". The rest is well specified: tool pin, export command and fixed flag values, arm files, labels by reference to anomaly phase 3: replay CLI, read-only export and blind validation (periodic, rate, new-stream) #457 round-2 §4, hashes posted before replay, and void and inconclusive cases. Not posted, as instructed.

Invariants

Check Result
Each requirement has a test red before / green after The new tests do not compile on origin/master (undefined: lateTracker), and they pass on the merged tree [T]
No behaviour change beyond the purpose Plain and -backlog-* exports are byte-identical to master (above) [T]
cmd/server read-only Untouched (git diff empty) [T]
New map[string]interface{} outside tests 0 added lines [T]. The new var flag interface{ flagged(...) bool } is a method interface, not a map
Hardcoded colours / XSS No public/ changes. scripts/check-xss-sinks.sh --diff origin/master → "no public/**/*.{js,html} changes to scan", exit 0 [T]
Fork guards github.repository == 'Kpa-clawbot/CoreScope': deploy.yml 9, release-fast-path.yml 1, on master and on the merged tree. .github/ is identical [T]
Closing keywords None in the commits or the PR body. closingIssuesReferences is empty [T]
Commit author All three commits: author and committer dborup, with the expected address [T]

Tests and mutants

Merged tree 13f479d0 (git merge-tree --write-tree origin/master fca73b3a, origin/master = 88f3be29), extracted with git archive.

Suite Result
cmd/anomaly go test ./... ok [T]
internal/anomaly ok [T]
cmd/server ok (69.6 s) [T]
cmd/ingestor ok (234 s) [T]
sh test-all.sh 272 files: 272 passed, 0 failed [T]
node test-frontend-helpers.js 725 passed, 0 failed [T]
E2E The PR has no E2E of its own, and no E2E is affected (no public/, cmd/server or cmd/ingestor change). Sanity run: test-e2e-playwright.js against a local Go server built from the merged tree, on e2e-fixture.db prepared as in CI (freshen, the Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL, corescope-migrate, seeds 2073, 199, 245 and 122): 154/157 passed, 3 skipped, 0 failed. The server was stopped by port (fuser -k) [T]
CI on head (run 40178366266, attempt 1) Every job passed. Publish Badges, Release Artifacts and Deploy Staging were skipped (fork / PR). Neither #256 nor #267 occurred [T]

Reviewer mutants, applied to late.go on the merged tree with the Late|Export|Backlog|Replay tests run (X1 also against the full cmd/anomaly suite, without my review file), then restored (cmp verified) [T]:

Mutant Result
X1 p.timestamp < o.timestamp + ?4 → <= survived (F6)
X2 scan margin Settle + 2·window → Settle + window killed (TestExportLateDeliverySummaryCountsTheScan)
X3 lateBin.add: drop the b.seen == 0 || guard on dmin killed (4 tests, including the sweep)
X4 window k−1..k (drop k+1) killed (TestLateTrackerBurst)
X5 see: drop elsewhere.Valid killed (8 tests)
X6 share bound <= 1 → < 1 killed (TestExportLateDeliveryIsOptIn)
X7 rows >= minRows → > killed (TestLateTrackerBurst)

The reproductions for F3, F4, F5 and F7 and the random-permutation privacy check are a review-only test file. It is not pushed.

Not verified

  • Behaviour on real backlogs, or on any data with real multi-observer overlap. No staging data was used, by design.
  • Whether common-cause outages (F3) or two-skewed-clock regions (F5) occur on the real network.
  • The author's mutants M1–M18. I ran my own instead.
  • That the author used no staging data, beyond what the diff and the artefacts show.
  • The referenced anomaly phase 3: replay CLI, read-only export and blind validation (periodic, rate, new-stream) #457 round-2 §4/§5 procedures (label457.py, calibrate.py), beyond the artifact-storm scoring rule.
  • Browser-visual validation. Not applicable: no UI change.

dborup and others added 2 commits October 25, 2026 09:44
…#538)

Review round 2 of #591 (F3-F6). The tests pin behaviour the definition
already has, so a change to it is a visible decision:

- a common-cause outage (two or three observers draining the same outage
  together) is never flagged, while one draining observer is;
- recall of a 40-GRP_TXT FIFO queue depends on the shared rows older than
  the gap that come with it (0/40 after 3-10 min, 40/40 after 20-40 min at
  6 shared rows a minute, 0/40 at 1 a minute);
- two clocks behind by different offsets flag 13 of 40 events of a live
  storm at share 0.5;
- a peer exactly at t + gap is not within the gap (kills the reviewer's
  mutant X1, < to <=).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review round 2 of #591 (F3-F5, F7, F8): the README and the code comment
state that a common-cause outage is never flagged, that recall falls for
large queues after short outages, that no share protects against two
clocks skewed by different offsets, that outages shorter than about two
gaps are never flagged, and that the share bound is compared in floating
point (exact for 0.5). No behaviour change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@dborup-agent

Copy link
Copy Markdown
Collaborator

Rapport — CS-pve-agent1 PR#591 runde-2 — head a643321

Review feedback addressed (commit a6433216; tests in c428a80b, docs in a6433216; PR description updated)

This round answers the review "Review — CS-pve-agent3 PR#591 — head fca73b3". The detector's behaviour is unchanged: late.go changed only in comments. The pre-registration is still a DRAFT in the PR description and has not been posted.

Evidence: [T] test or command I ran, [A] analysis, [K] code, config or issue read.

  1. F1 (comparative criterion). Pre-reg §5 now defines N_art, N_real, EA_art, EB_art and L (non-artifact storms that arm A detects and arm B does not). They use the anomaly phase 3: replay CLI, read-only export and blind validation (periodic, rate, new-stream) #457 round-2 §5 matching rule, unchanged. §6 adds two criteria that a go needs on top of the absolute bars:

    • C1: N_art ≥ 1 and EB_art < EA_art.
    • C2: L = 0, with a tolerance of zero storms. This implies that arm B's recall is at least arm A's.
    • If N_art = 0 or EA_art = 0, the benefit is unmeasured: the round is inconclusive and counts as a no-go. C2 and the bars are still reported, as a safety result only.
    • §6 now says what a go licenses: on this network in E, with the fixed values, the guard reduced episodes on anomaly phase 3: replay CLI, read-only export and blind validation (periodic, rate, new-stream) #457-labelled artifacts compared with the same rules unguarded, lost no non-artifact storm, and met the bars.
    • It also says what a go does not license: defaults, other rules, windows, values or networks, backlog recall, or common-cause outages.
    • Arm A is no longer called "a replication". It is the comparator, and its own bars do not decide. [K]
  2. F2 (judgement left after data).

    • (a) V is fixed as [2026-11-02T00:00Z, 2026-11-09T00:00Z), with C as the first day and E as the following 6 days. The export and replay commands carry these dates. V is disjoint from anomaly: periodic rule targets channel spam bots (not wardriving) — exclusions + injection-based validation #474 round 4, whose V ends at the latest on 2026-11-01T00:00Z. V starts one day later, so the scan margin (V.start − 30 s − 2 min) does not reach that round's V either. The round is void if the text is posted on or after V.start, or if data from V was seen before posting; there is no shifting.
    • (b) The recall comparison is now C2: comparator arm A, unit non-artifact storms, tolerance 0.
    • (c) Capacity: the export runs at -max-rows 50000000. It runs once more at 100000000 (maxExportRows, export.go:54), and a failure there voids the round. The bin cap still voids it as well.
    • (d) <HEAD SHA> is defined as the PR anomaly: backlog signal for late-delivered observations #591 head commit that the last review reviewed: a branch commit, not a merge commit and not master. If it cannot be built, the round is void.
    • <HEAD SHA> is now the only placeholder left. [K]
  3. F3 (common-cause outage never flagged). This is stated in the PR's Limitations, in pre-reg §2, in the README and in the late.go comment. It is pinned by TestLateDeliveryMissesACommonCauseOutage, which uses a FIFO queue at 1 row/s:

    Network 1 drainer 2 together 3 together
    background every 60 s, 40-min outage, 25 queue-only GRP_TXT 25/25 0/25 0/25
    background every 10 s, 20-min outage, 40 queue-only 40/40 0/40 0/40
    background every 10 s, 40-min outage, 40 queue-only 40/40 0/40 0/40

    Live events: 0 flagged in every case [T]. These reproduce the review's figures, except one: a single drainer of the 60 s network flags 25/25 here, where the review had 20/25. My queue is FIFO, and the review did not give its delivery order [A].

  4. F4 (recall for large queues). This is stated in Limitations, pre-reg §2 and the README. The sweep sentence "pessimistic for a busier network" is now limited to long outages. It is pinned by TestLateDeliveryRecallOfALargeQueue: 40 queue-only GRP_TXT, FIFO, 5 rows/s. At 6 shared rows/min the flag gives 0/40 after outages of 3, 5 and 10 min and 40/40 after 20 and 40 min. At 1 shared row/min it gives 0/40 after outages of 3–40 min. These are the review's figures [T].

  5. F5 (share 0.5 is not a protection). The rationale in §2 now says that 0.5 flagging no live event is a property of the sweep, not a protection. The Limitations and the README state that two clocks behind by different offsets can pass the spread condition at any share. §2 adds that such a storm can lose its episode in arm B, and C2 counts that loss. It is pinned by TestLateDeliveryTwoSkewedClocksDefeatTheShare: a live storm of 40 GRP_TXT, heard by O, one third also by an observer 3 min behind and one third by one 7 min behind. It gives 13/40 flagged, the review's figure [T].

  6. F6 (exclusive t + gap bound). TestLateQueryPeerAtExactlyTheGapIsNotWithin runs lateQuery on a fixture with two cases:

    • The only later peer sits exactly at t + gap, and an older peer sits at t − 3 min. elsewhere is t − 180, so the row is already seen.
    • A peer sits at t + gap − 1 s. elsewhere is that peer, so the row is not already seen.

    The reviewer's X1 (< → <=) is now killed (MF6 below) [T].

  7. F7 (nit, float share bound). Documented, not changed, as in "either document it". The flagged comment, the README and the Limitations state that the comparison is exact for multiples of 1/2^k, such as the pre-registered 0.5. They also state that 0.07 · 100 evaluates to 7.000000000000001, so 7 seen rows out of 100 is not a burst. I checked that value with a go run [T]. I made no code change because the brief asks for no functional change beyond F6 [A].

  8. F8 (nit, short outages). One line in the Limitations, the README and pre-reg §2: an outage shorter than about 2·gap (4 min) is never flagged [A].

No finding was rejected.

Tests

The new tests (cmd/anomaly/late_limits_538_test.go) pin behaviour the code already has. The review asks for no functional change, so they could not start red on unmutated code. Each one was shown red against a mutant of the property it pins:

  • On c428a80b (fca73b3a plus the test file only; late.go unchanged) all four pass.
  • On the commit they fail under the mutants below. [T]
Mutant (in late.go, restored with git checkout, cmp verified) Finding Killed by
MF3 peers within the gap ignored: p.timestamp < o.timestamp + ?4 → p.timestamp <= o.timestamp - ?4 (an attempted fix that would flag common-cause outages) F3 TestLateDeliveryMissesACommonCauseOutage, TestLateQueryPeerAtExactlyTheGapIsNotWithin, TestExportLateDeliverySummaryCountsTheScan, TestLateDeliverySyntheticSweep
MF4 queue-only rows leave the share's denominator (rows counted only when elsewhere.Valid) F4 TestLateDeliveryRecallOfALargeQueue, TestLateTrackerBurst
MF5 spread must exceed 2·gap: w.dmax-w.dmin >= l.gap → > 2*l.gap F5 TestLateDeliveryTwoSkewedClocksDefeatTheShare, TestLateTrackerBurst
MF6 = reviewer's X1: p.timestamp < o.timestamp + ?4 → <= F6 TestLateQueryPeerAtExactlyTheGapIsNotWithin only (it survived the whole suite before)

F1, F2, F7 and F8 changed only text, so no mutant applies to them. A candidate MF5′ (MAX → MIN) was not killed by the F5 test, because each of O's rows there has only one skewed peer. I replaced it with MF5 [T].

Local runs (head a643321)

origin/master (88f3be29) is already an ancestor of the branch, so no merge was needed [T].

Suite Result
cmd/anomaly go test -count=1 ./... ok [T]
internal/anomaly ok [T]
cmd/server ok (69.8 s) [T]
cmd/ingestor ok (223.5 s) [T]
sh test-all.sh 271 passed, 0 failed (271 files) [T]
node test-frontend-helpers.js 725 passed, 0 failed [T]
gofmt -l, go vet (cmd/anomaly) clean [T]
E2E No E2E is affected: there is no public/, cmd/server or cmd/ingestor change. As a sanity run, I ran test-e2e-playwright.js against a local Go server built from this head, on a scratch copy of e2e-fixture.db prepared as in CI (freshen, the Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL, corescope-migrate, seeds 2073, 199, 245 and 122). Result: 154/157 passed, 3 skipped, 0 failed. The server was stopped by port (fuser -k) [T]

CI per job (head a643321)

Run 40182335343, attempt 1, conclusion success [T]:

Job Result
Go Build & Test pass
Go Ingestor Tests pass
Go Libraries & CLI pass
Go Server Tests 1/3, 2/3, 3/3 pass
JavaScript Unit Tests & Lint pass
Script & Container Unit Checks pass
Playwright E2E 1/3, 2/3, 3/3 pass
Verify E2E Shards & Coverage pass
Verify Go Test Assignments pass
Validate Docker Image pass
Build & Publish Docker Image pass
Publish Badges & Summary, Release Artifacts, Deploy Staging skipped (fork / PR)

No reruns were needed. Neither #271 nor #301 occurred.

Invariants

  • The diff against fca73b3a touches only cmd/anomaly/ (README, late.go comments, one new test file). internal/, cmd/server, cmd/ingestor, public/ and .github/ are unchanged [T].
  • No new map[string]interface{} [T].
  • Commits are authored by dborup <kontakt@meshview.dk>. There was no rebase, amend or force-push [T].
  • No closing keywords; closingIssuesReferences is empty. The PR is still a draft [T].
  • No staging or prod access, no API key, no upstream writes. Only synthetic data and the committed e2e fixture were used [K].

Not verified

@dborup-agent

Copy link
Copy Markdown
Collaborator

Review — CS-pve-agent3 PR#591 — head a643321

Dom: APPROVE med nits

This is a re-review of round 2 against my review of fca73b3a. F1–F8 are resolved. The code change since fca73b3a is comments in late.go, one new test file and the README, so the detector is unchanged. The draft pre-registration is nearly mechanical. Four small points (N1–N4) still leave a choice to whoever scores the round. They are cheap to fix now and impossible to fix once the text is posted, so please fold them into the draft before the coordinator posts it.

This is an independent, read-only review. Evidence: [T] test or command I ran, [A] analysis, [K] code, config or issue read.

Findings

# Severity Where Finding
N1 Low Pre-reg §6 The verdict categories overlap, with no stated order. There are three cases. (a) N_art ≥ 1 and EA_art = 0 is "inconclusive" by the zero-episode rule, and also "no-go", because EB_art ≥ EA_art holds trivially. (b) With zero non-artifact storms, recall is undefined. That is a failed bar (no-go) and also "inconclusive". (c) With N_art = 0, a failed bar gives both labels. Every reading means "no claim", but which label the round reports would be chosen after the data. Suggest a fixed order: void, then inconclusive, then no-go, then go. [K][A]
N2 Low Pre-reg §1 Capacity, §3 Replay capacity: per arm or both arms together? "#457 round 2 §1a" also covers the replay step: if CoverageReduced is set, the six count caps are doubled, up to 4 times. #473 said "applies per run". This draft does not say. If only one arm's caps are doubled, the arms differ in limits, which contradicts "Nothing else differs" (§3). Suggest that the caps are doubled for both arms together and both replays are rerun. [K]
N3 Low Pre-reg §5 The scoring code and the episode selection are not pinned. The published #457 metrics457.py selects rate episodes with rule.startswith("p457_rate"), so arm B's p538_rate_* rules would score zero episodes if it were reused unchanged. EA_art, EB_art and L also need new code. Suggest two additions. First, "an arm's rate episodes are the episodes of the two rules in its config file". Second, post the scoring script's sha256 together with the label sha256, before either replay runs, as the #473 results did after the fact (0f022f65…). [K]
N4 Low Pre-reg §1 Snapshot No data-coverage check. The late flag does not refuse a database whose first observation is later than the scan start. The #473 flag's lookback check (firstObservationQuery, backlog.go) is not used by -late-*. The draft also has no retention rule. If packet retention prunes the early days of V before the snapshot is taken (after 2026-11-09), C and part of E are silently partial. Suggest voiding the round if SELECT MIN(timestamp) FROM observations on the snapshot is later than V.start − 30 s − 2 min, and reporting the retention setting, as #474 round 4 does. [K][A]
N5 Nit Pre-reg §1, V row The disjointness premise is conditional. #474 round 4's V "begins at the first present whole day at or after T0" (T0 = 2026-10-25T00:00Z) and runs for up to 7 days. It therefore ends by 2026-11-01T00:00Z only if 2026-10-25 is a present day. If that day or later days are not present, that V shifts and can overlap [2026-11-02, 2026-11-09). No rule here depends on disjointness, and the real integrity guard, "no data from V seen before posting", is intact. The sentence states as fact something that is not yet known, though. Suggest rewording to "disjoint if #474 round 4's V starts on T0; an overlap does not void this round", or adding the overlap as a void condition. [K]
N6 Nit Author's report "origin/master (88f3be29) is already an ancestor of the branch" is not correct. git merge-base origin/master a6433216 = bb91292a [T]. It has no effect here: git diff bb91292a origin/master -- cmd/anomaly internal/ is empty, so the anomaly binary built from the head equals the one built from the merged tree. CI ran on the PR merge ref [T]. The author's local suite runs were on the unmerged head, though. Mine below are on the merged tree.

Answers to the re-review points

  • F1 (comparative criterion, what a go licenses): resolved.
    • §5 defines N_art, N_real, EA_art, EB_art and L. Each uses the anomaly phase 3: replay CLI, read-only export and blind validation (periodic, rate, new-stream) #457 round-2 §5 matching rule. I checked that rule: an episode "is on" a storm when it has the same payload type and [activation, end] overlaps [start − 5 min, stop + 5 min]. A storm is detected when it has at least one such TP episode. That is mechanical, and it matches episodes_on_artifacts in metrics457.py [K].
    • C1 (N_art ≥ 1 and EB_art < EA_art) and C2 (L = 0) are both required for a go. The bars alone no longer suffice.
    • The paragraph on what a go licenses is explicit. A go covers this network, this E, these values and these two rules. It covers no defaults, no backlog recall and no common-cause outages.
    • The "flag does nothing" case of round 1 now fails C1, because EB_art = EA_art [K][A].
    • Only label precedence remains (N1).
  • F2 (judgement after data): resolved.
    • (a) V is fixed as [2026-11-02T00:00Z, 2026-11-09T00:00Z), which is at or after 2026-11-01T00:00Z. C and E are derived from it, and the commands carry literal dates. The round is void if the text is posted after V.start or after data from V was seen. Nothing shifts. The premise about anomaly: periodic rule targets channel spam bots (not wardriving) — exclusions + injection-based validation #474 r4 is conditional (N5).
    • (b) C2 compares with arm A, at a tolerance of zero non-artifact storms [K].
    • (c) The capacity stop rule is explicit: 50M, then once at 100M (maxExportRows, export.go), otherwise the round is void. The bin cap also voids it. Replay capacity is open (N2).
    • (d) <HEAD SHA> is the branch commit that the last review reviewed, not a merge commit and not master. An unbuildable commit voids the round. It is the only placeholder left [K].
  • F3 (common-cause outage): resolved.
    • It is documented in the PR Limitations, pre-reg §2 (including "episodes that such outages cause stay in arm B as in arm A"), the README and the late.go header [K].
    • It is pinned by TestLateDeliveryMissesACommonCauseOutage: 1 drainer flags 25/25 and 40/40; 2 or 3 drainers flag 0; no live events are flagged [T].
    • Two mutants that "fix" the miss are killed by this test: my Y8 (MAX → MIN, earliest peer) and the author's MF3 [T].
    • The 25/25 versus my 20/25 is explained by delivery order: their queue is FIFO, and my round-1 reproduction used a different order. Both show the same limit [A].
  • F4 (recall for large queues): resolved. The text is in the PR, pre-reg §2 and the README. It is pinned by TestLateDeliveryRecallOfALargeQueue, which reproduces my round-1 figures [T].
  • F5 (share 0.5 is not a protection): resolved.
    • §2 now calls the sweep result "a property of the sweep, not a protection" [K].
    • The Limitations and the README give the 13/40 case [K].
    • §2 notes that C2 counts the resulting loss [K].
    • It is pinned by TestLateDeliveryTwoSkewedClocksDefeatTheShare, which requires exactly 13 [T].
  • F6 (X1): resolved.
    • TestLateQueryPeerAtExactlyTheGapIsNotWithin checks two boundary cases: a peer exactly at t + gap, which is not within the gap, and a peer at t + gap − 1 s, which is within it [T].
    • X1 (< → <=) is now killed by that test [T].
    • My Y1 (window t + gap/2) is killed by the same test [T].
  • F7, F8 (nits): resolved as text. The float bound is documented in the flagged comment, the README and the Limitations. The "< 2·gap outage" line is in the Limitations, the README and §2 [K].
  • Final check: does the draft leave anything to judgement after the data? Almost nothing. The remaining choices are N1 (which label), N2 (per-arm vs joint replay caps), N3 (episode selection and the scoring code) and N4 (what to do with a pruned V). Only N2 could, in principle, change an arm's episodes. None can turn a no-go into a go under any reasonable reading. All four are one-line fixes. Not posted, as instructed.

Invariants

Check Result
Each requirement has a test red before / green after On origin/master with the PR's test files, the tests do not compile (opt.LateWindow undefined) [T]. They pass on the merged tree [T]. The four round-2 tests pin existing behaviour, so their red is shown by mutants. The author shows MF3–MF6; I show X1, MF3, Y1, Y5 and Y8 below [T]
No behaviour change beyond the purpose late.go changed only in comments since fca73b3a [K]. The plain and -backlog-* byte-identity verified in round 1 still applies [A]
cmd/server read-only git diff origin/master <merged> -- cmd/server cmd/ingestor public .github internal is empty [T]
New map[string]interface{} 0 added lines [T]
Hardcoded colours / XSS No public/ change. scripts/check-xss-sinks.sh --diff origin/master on the head gives "no public/**/*.{js,html} changes to scan", exit 0 [T]
Fork guards github.repository == 'Kpa-clawbot/CoreScope': deploy.yml has 9 and release-fast-path.yml has 1, on master and on the merged tree [T]
Closing keywords None in the commits or the PR body. closingIssuesReferences is empty [T]
Commit author All five commits have author and committer dborup, with the expected address [T]
Head unchanged git ls-remote gave a6433216… before and after the review [T]

Tests and mutants

The merged tree is 0ef0c83a (git merge-tree --write-tree origin/master a6433216…, with origin/master = 88f3be29), extracted with git archive into scratch.

Suite Result
cmd/anomaly go test -count=1 ./... ok [T]
internal/anomaly ok [T]
cmd/server ok (69.6 s) [T]
cmd/ingestor ok (228.5 s) [T]
gofmt -l, go vet (cmd/anomaly) clean [T]
sh test-all.sh 272 files: 272 passed, 0 failed [T]
node test-frontend-helpers.js 725 passed, 0 failed [T]
E2E The PR has no E2E of its own, and none is affected. Sanity run: test-e2e-playwright.js against a local Go server built from the merged tree, on e2e-fixture.db prepared as in CI (freshen, the Kpa-clawbot#1486/Kpa-clawbot#1791 seed SQL from deploy.yml, corescope-migrate, seeds 2073, 199, 245 and 122). Result: 154/157 passed, 3 skipped, 0 failed. The server was stopped by port (fuser -k) [T]
CI on head (run 40182335343, attempt 1, PR merge ref) All 15 jobs succeeded: Go Build & Test, Go Ingestor, Go Libraries & CLI, Go Server 1–3/3, JS Unit & Lint, Script & Container, Playwright 1–3/3, Verify E2E Shards & Coverage, Verify Go Test Assignments, Validate Docker Image, Build & Publish Docker Image. Publish Badges, Release Artifacts and Deploy Staging were skipped (fork / PR). Neither #256 nor #267 occurred [T]

Mutants were applied to late.go in a separate copy of the merged tree. Each was run against the full cmd/anomaly suite and then restored (cmp verified) [T]:

Mutant Result
X1 (round 1) p.timestamp < o.timestamp + ?4 → <= killed, only by TestLateQueryPeerAtExactlyTheGapIsNotWithin (it survived in round 1)
MF3 (author's, re-run) … + ?4 → p.timestamp <= o.timestamp - ?4 killed by 4 tests, including TestLateDeliveryMissesACommonCauseOutage
Y1 the "within" bound at t + gap/2 killed by TestLateQueryPeerAtExactlyTheGapIsNotWithin
Y2 window bins k−2..k+2 instead of k−1..k+1 killed by TestLateTrackerBurst
Y5 already seen needs d ≥ 2·gap killed by 6 tests, including TestLateDeliveryTwoSkewedClocksDefeatTheShare
Y6 -late-min-rows counts only already-seen rows killed by TestLateTrackerBurst
Y8 MAX(p.timestamp) → MIN (earliest other observer) killed by 4 tests, including TestLateDeliveryMissesACommonCauseOutage
Y9 spread must be ≥ gap + 60 s killed by TestLateTrackerBurst

Not verified

  • Behaviour on real data, or whether common-cause outages and two-skewed-clock regions occur on the real network. No staging or prod data was used.
  • Whether 2026-10-25 is a "present" day for anomaly: periodic rule targets channel spam bots (not wardriving) — exclusions + injection-based validation #474 round 4, which is what N5 depends on. That is not knowable yet.
  • The full label457.py and calibrate.py procedures, beyond the rate-label and rate-matching parts that C1 and C2 reuse.
  • The author's MF4 and MF5 and the M1–M18 mutants. I re-ran only MF3 and X1, plus my own mutants.
  • That the author saw no staging data, beyond what the diff and the artefacts show.
  • Browser-visual validation. Not applicable: there is no UI change.

@dborup
dborup marked this pull request as ready for review October 25, 2026 10:30
@dborup
dborup merged commit f7e13bc into master Oct 25, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants