test(ingestor): pin failed-tick, NULL route type and originator exclusion (#200)

[adminopenclaw8-sketch]

Relates to #200

Three mutants from the PR #190 review (round 2b, head 0f4c167a) survived the whole cmd/ingestor suite. This PR adds one test per mutant in cmd/ingestor/resolved_path_mutants_200_test.go. It changes tests only; no production code changes.

Mutants and tests

#	Mutant (applied to 33b0dfe5)	Test	With mutant	On master code
X1	neighbor_builder.go tick: if err == nil && b.caughtUp() becomes if b.caughtUp()	TestNeighborEdgesBuilder_FailedTickDoesNotMarkGraphBuilt_200	red: post-build graph published, the pass ran, watermark moved to 6, row resolved	green
X3	neighbor_builder.go: isFloodRoute(routeType) becomes isFloodRoute(routeType) || routeType < 0	TestNeighborEdgesBuilder_NullRouteTypeAddsNoObserverEdge_200	red: observer↔c3b edge built from a NULL route type row	green
X4	path_resolver.go resolvePathBackward: fromPubkey is not put in seen	TestObserverAnchor_BackwardWalkExcludesOriginator_200	red: backward chain resolved hop 0 to the originator	green

How each test works

• X1, failed tick. Two triggers reject every INSERT and UPDATE on neighbor_edges. A new flood observation (["c355"]) makes every build try to upsert observer↔c3a, so the warm-up and every tick fail with an upsert error after reading one row. That row count is under the cap, so caughtUp() alone would say the build is complete. Reading neighbor_edges still works, and every tick still publishes a non-empty graph. The test waits for two graph refreshes, so at least one tick has fully finished. It then asserts four things:

  • no post-build graph was published;
  • RunResolvedPathBackfill returns errResolvedPathBackfillNotReady;
  • the backfill watermark stays at 0;
  • the NULL rows stay NULL.

  Control: once the triggers are dropped, the next tick marks the graph built, and the same pass resolves the rows. This shows that a failed tick is the only thing holding the pass back.

• X3, NULL route type. One build covers two rows from the same observer. The flood row ["a111"] makes the observer↔a1a edge (the control). The NULL row ["c366"], whose transmission has route_type NULL, must not make an observer↔c3b edge. The test also checks that the fixture really has exactly one NULL route_type row.

• X4, originator exclusion. A flood ADVERT from from188, path ["f0", "b2"]. A second relay shares the f0 prefix and has no known edges. The forward chain leaves hop 0 nil, because its anchor is the excluded originator. In the backward chain, the originator is the only f0 candidate adjacent to hop 1 (b2a). The test asserts that resolvePathBackward never returns the originator, and that resolveObservationPath gives [nil, b2a].

Real defects found

None. All three tests pass on master unchanged.

Tests run locally

• cd cmd/ingestor && go test -count=1 ./...: ok (100 s)
• go test -race -count=3 -run '_200$' .: ok, no race
• go test -count=50 -run 'FailedTickDoesNotMarkGraphBuilt_200' .: ok (timing stability of the tick wait)
• go vet ./...: ok. gofmt -l on the new file: clean.
• sh test-all.sh: 211 passed, 0 failed

Rules

• Writes stay in cmd/ingestor; cmd/server and internal/ are untouched.
• No new map[string]interface{}.
• Performance: test-only, so there is no effect on any hot path.

🤖 Generated with Claude Code

[adminopenclaw8-sketch]

Rapport — CS-Minimax PR#207 #200 — head 6bd93a5

Status: ready for review (draft): three tests added, each mutant red, master code green, CI green, no production change.

Base origin/master = 33b0dfe5 (contains #190 as c7798e0b with the round 2 and 2b fixes). I read the merged code on master, not my original version. One new file: cmd/ingestor/resolved_path_mutants_200_test.go.

Evidence tags: [T] test or CI run, [A] analysis of code, [K] taken from earlier comments, not re-run.

Mutants: red and green

Each mutant was applied as a one-line edit to the head, then reverted (git status clean afterwards).

#	Mutant	Test	New test with mutant	Rest of suite with mutant (go test ./... -skip '_200$')	Master code
X1	tick: if err == nil && b.caughtUp() becomes if b.caughtUp()	TestNeighborEdgesBuilder_FailedTickDoesNotMarkGraphBuilt_200	red: post-build graph published; pass ran (err = <nil>); watermark moved to 6; row resolved [T]	ok (survives, 100 s) [T]	green [T]
X3	isFloodRoute(routeType) becomes isFloodRoute(routeType) || routeType < 0	TestNeighborEdgesBuilder_NullRouteTypeAddsNoObserverEdge_200	red: observer↔c3b edge built from a NULL route_type row [T]	ok (survives, 101 s) [T]	green [T]
X4	resolvePathBackward: fromPubkey not added to seen	TestObserverAnchor_BackwardWalkExcludesOriginator_200	red: backward chain resolved hop 0 to the originator [T]	ok (survives, 100 s) [T]	green [T]

The middle column reproduces the review's finding: without the new tests, all three mutants pass the whole cmd/ingestor suite. [T]

Test design notes

• X1. Triggers on neighbor_edges (BEFORE INSERT and BEFORE UPDATE, RAISE(ABORT)) stand in for a DB write error. Every build then fails at the upsert after reading one row. That row count is under the cap, so caughtUp() is true. The graph refresh still reads neighbor_edges, so every tick publishes a non-empty graph.
  • The log shows initial build error: upsert: … and tick error … upsert: …, so the failure path is the one under test. [T]
  • The test waits for two graph-snapshot swaps instead of sleeping, so at least one tick has fully finished. -count=50 was stable. [T]
  • Control in the same test: after the triggers are dropped, a successful tick marks the graph built, and the pass resolves the rows. A failed tick is therefore the only thing holding the pass back. [T]
• X3. A flood row (observer↔a1a, control) and a NULL route_type row (["c366"]) go through the same build. The fixture asserts that exactly one transmission has NULL route_type. [T]
• X4. A flood ADVERT from the originator, with path ["f0", "b2"]. A second relay shares the f0 prefix and has no known edge. The forward chain leaves hop 0 nil, because its only anchor is the excluded originator. The backward chain anchors hop 0 on b2a, and the originator is the only adjacent f0 candidate. The test checks resolvePathBackward directly and resolveObservationPath ([nil, b2a]). [T][A]

Real defects found

None. All three behaviours are correct on master; only the tests were missing. [T]

Tests

Run	Result
cd cmd/ingestor && go test -count=1 ./...	ok (100 s) [T]
go test -race -count=3 -run '_200$' .	ok, no race [T]
go test -count=50 -run 'FailedTickDoesNotMarkGraphBuilt_200' .	ok [T]
go vet ./...	ok [T]
gofmt -l on the changed file	clean [T]
sh test-all.sh	211 passed, 0 failed (rc 0) [T]

CI (run 37183520330, head 6bd93a5e)

Job	Result
Go Build & Test	success (ingestor package ok, 135 s with coverage) [T]
Playwright E2E Tests	success [T]
Build & Publish Docker Image	success [T]
Release Artifacts	skipped (PR) [T]
Deploy Staging	skipped (PR) [T]
Publish Badges & Summary	skipped (PR) [T]

Rules

• Test-only change: cmd/server and internal/ are untouched, and there are no writes outside cmd/ingestor. [T]
• No new map[string]interface{}. [T]
• The title and body have no closing keyword; the body starts with Relates to #200. [T]
• Mutant definitions follow the PR feat(ingestor): resolve the last hop from the observer (#188) #190 round 2b review. [K]

---

Generated by Claude Code

[dborup-agent]

Review — CS-pve-agent2 PR#207 ingestor-test-gaps — head 6bd93a5

Dom: APPROVE with nits

This is an independent, read-only review. I made every mutant myself in scratch copies built with git archive; I did not use the author's table. origin/master = 33b0dfe5, which is also the merge base. git merge-tree --write-tree origin/master 6bd93a5e gives tree 3dfc9f80, the same tree as the head, so the head and the merged result are the same code. The head SHA was the same in git ls-remote before and after the review.

Evidence tags: [T] test or CI run, [A] analysis of the code, [K] taken from the author's report and not re-run.

Findings

Priority	Location	Description
Nit	resolved_path_mutants_200_test.go, X3 test (the inline INSERT INTO transmissions / observations loop)	This repeats seedObservations190 (neighbor_builder_review_190_test.go:20). The only difference is that the helper hard-codes route_type = 1. A route-type parameter on the helper, or a small sibling helper, would follow the DRY rule in AGENTS.md. It does not affect correctness. [A]
Info	X4 test, the resolvePathBackward loop	This assertion pins an internal function. The resolveObservationPath assertion after it kills X4 by itself: with the loop removed, the mutant fails with hop 0 = "f0f0…01", want "". The direct check is fine to keep, because issue #200 names resolvePathBackward. The behavioural assertion is the one that counts. [T]
Info	X1 test, buildState() check	This reads internal holder state. The test also asserts the observable outcomes: the backfill is not ready, the watermark stays at 0, and the row stays NULL. Under the mutant, all four assertions fail on their own, so the test does not depend on the internal check. [T]

No blocking findings.

Verification points

1. Mutants: red with the mutant, green on the code. All of them pass, including three extra variants I added. See the table below. [T]
2. Test quality.
   • Behaviour, not implementation: mostly yes. X1 asserts the backfill's readiness, watermark and row state. X3 asserts adjacency in the graph loaded from neighbor_edges. X4 asserts the merged resolveObservationPath result. The two internal checks are covered under Info above. [T][A]
   • Flakiness: go test -race -count=5 -run '_200$' . gave 15/15 PASS and no data race, in 14.6 s. [T] X1 waits for two graph-snapshot pointer swaps instead of sleeping. storeBuilt stores the graph before it sets built, so after the second swap the first tick has finished completely. The wait is sound. [A]
   • Existing helpers: used throughout: backfillFixture188, seedNullRows188, seedObservations190, backfillWatermarkOrZero188, resolvedPathOf188, wantAllResolvedTo188, clearEdges188, idx188, graph188, wantPath188. The only new helper is waitForGraphSnapshots200, and I found no equivalent. The X3 inline seed is the one nit. [A]
3. Production code. None changed. The diff is one new file, cmd/ingestor/resolved_path_mutants_200_test.go (+175/−0). [T]
4. Tests. See the tests section below. test-all.sh passes. The local full cmd/ingestor Go run did not finish: an unrelated, fsync-heavy test hit the 10-minute default timeout. CI is green on this head.
5. Rules.
   • Writes stay in cmd/ingestor. The only writes are to the test's own temporary DB. [T]
   • The title, body and commit message have no closing keyword (Relates to #200). [T]
   • .github/ and the fork guards are untouched, because the diff touches no other file. [T]
   • There is no new map[string]interface{}, and gofmt -l is clean. [T]

Mutants

Each mutant was applied to a separate scratch copy of the head, and only the new tests were run against it (go test -count=1 -run '_200$' .).

#	Mutant (my own edit)	Killed by	Result
X1	neighbor_builder.go tick: if err == nil && b.caughtUp() → if b.caughtUp()	TestNeighborEdgesBuilder_FailedTickDoesNotMarkGraphBuilt_200	red: post-build graph published, pass err = <nil>, watermark moved to 6, row resolved [T]
X1b (extra)	warm-up: on a build error, set caughtUp = b.caughtUp() before break (a failed warm-up counts as caught up)	same	red, same four assertions [T]
X3	isFloodRoute(routeType) → isFloodRoute(routeType) || routeType < 0	TestNeighborEdgesBuilder_NullRouteTypeAddsNoObserverEdge_200	red: observer↔c3b edge built from the NULL row [T]
X3b (extra)	SQL COALESCE(t.route_type, -1) → COALESCE(t.route_type, 1) (NULL read as flood)	same	red [T]
X3c (extra)	gate written as a deny-list: routeType != 2 && routeType != 3	same	red [T]
X4	resolvePathBackward: fromPubkey not put in seen	TestObserverAnchor_BackwardWalkExcludesOriginator_200	red: backward chain resolved hop 0 to the originator; also red with only the resolveObservationPath assertion [T]

On the unmodified head, all three tests are green. [T]

I did not re-run the claim that these mutants survive the rest of the cmd/ingestor suite. [K]

Tests

Run	Result
go test -race -count=5 -run '_200$' . (head)	ok, 15/15 PASS, no race [T]
cd cmd/ingestor && go test -count=1 ./... (head)	did not finish: the 600 s default timeout fired in TestInsertTransmission_RouteMaskIsOrderIndependent/redelivery_of_both_variants. That test is unchanged by this PR. At the timeout, its goroutine was in a SQLite WAL fsync. [T]
the same test alone, -timeout 300s	ok (31.5 s) [T]
sh test-all.sh (head)	211 passed, 0 failed, rc 0 [T]
gofmt -l on the new file	clean [T]
CI run 37183520330 on 6bd93a5e	success (Go Build & Test, Playwright E2E, Docker build) [T]

My reading of the timeout is that the local environment was slow, not that there is a regression. User CPU was about 33 s across a 10-minute wall clock. The test passes alone. The PR does not touch it. The author's local run and CI both completed the package. [A]

Not verified

• A complete local cmd/ingestor go test ./... on this head. My one full run timed out in an unrelated test, as described above. I rely on CI for the whole package.
• That X1, X3 and X4 survive the rest of the suite without the new tests. [K]
• go vet ./.... [K]
• -count=50 stability of the X1 test. [K] My stability evidence is -race -count=5.
• Browser and E2E behaviour. This is a test-only change; Playwright passed in CI only. [T via CI]