Skip to content

test(ingestor): pin the loser's re-parented observation headers in the merged route_mask (#324) - #328

Merged
dborup merged 1 commit into
masterfrom
codex/issue-324-route-mask-fifth-term
Oct 7, 2026
Merged

dborup merged 1 commit into
masterfrom
codex/issue-324-route-mask-fifth-term

Conversation

@dborup-agent

@dborup-agent dborup-agent commented Oct 6, 2026 •

Copy link
Copy Markdown
Collaborator

Relates to #324

Follow-up to #295 (#287): pins the fifth evidence term of recomputeMergedRouteMask, the header bits of the loser's observations after UPDATE OR IGNORE observations re-parents them onto the survivor.

Plan

  1. Add one test with the RZ1 shape from the issue, where the loser's re-parented observation header is the only evidence for the DIRECT bit.
  2. Show it green on master's code and red under mutant N2.
  3. Test-only: no production change.

Change

cmd/ingestor/hash_migrate_route_mask_287_test.go: new TestContentHashMigration_MergeRecomputesBitFromLoserReparentedObservation_324.

Side route_mask route_type Observation
Survivor (id 130) FLOOD (computed) FLOOD FLOOD header, observer o1, path ["aa"]
Loser (id 131) NULL FLOOD DIRECT header, observer o2, path ["bb"] (survives the move)

The test asserts:

  • the merged mask is DIRECT|FLOOD (6);
  • the survivor holds both observations;
  • the backfill leaves the mask unchanged.

Mutant N2

N2 moves recomputeMergedRouteMask above the UPDATE OR IGNORE observations move, so the scan sees only the survivor's own rows.

Tree New test Full cmd/ingestor suite
this branch (production code = master de6e83b4) PASS, mask 6 ok
N2 FAIL: merged route_mask = {2 true}, want DIRECT|FLOOD = 0110 FAIL, only this test

Checks

  • cmd/server untouched (stays read-only); no new map[string]interface{}.
  • No frontend change; bash scripts/check-xss-sinks.sh --diff origin/master is clean.
  • Fork guards unchanged: 9 in deploy.yml, 1 in release-fast-path.yml.

Performance

Test-only; no hot path touched.

🤖 Generated with Claude Code

…e merged route_mask (#324)

recomputeMergedRouteMask ORs five evidence terms. The fifth, the header
bits of the loser's observations after the UPDATE OR IGNORE move
re-parents them onto the survivor, had no test: mutant N2 (recompute
above the move, so the scan sees only the survivor's own rows) survived
the full cmd/ingestor suite.

Add the RZ1 shape from the issue: survivor computed FLOOD with one FLOOD
observation; loser route_mask NULL, route_type FLOOD, one surviving
DIRECT observation. That header is the only DIRECT evidence, so the
merged mask must be DIRECT|FLOOD (6). Under N2 it is FLOOD (2).

Test-only.

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

Copy link
Copy Markdown
Collaborator Author

Rapport — CS-pve-agent2 PR#328 #324 — head 4912821

Status: Done. The test pins the fifth route_mask merge evidence term: green on head, red under N2. Test-only, all CI jobs green, still a draft.

Evidence: [T] = run locally on this head, [A] = analysis / code reading, [K] = CI check on this head.

Requirements

# Requirement Test Mutant Evidence
1 RZ1 shape: the loser's re-parented observation header is the only evidence for a bit TestContentHashMigration_MergeRecomputesBitFromLoserReparentedObservation_324 (cmd/ingestor/hash_migrate_route_mask_287_test.go). Survivor 130 is computed FLOOD with a FLOOD observation (o1, ["aa"]). Loser 131 has route_mask NULL, route_type FLOOD and one DIRECT observation (o2, ["bb"]). — [A] DIRECT comes only from the loser's observation header: both route_types are FLOOD, the survivor's stored mask is FLOOD and the loser's mask is NULL.
2 Red under N2, green on master same N2: recomputeMergedRouteMask moved above UPDATE OR IGNORE observations [T] Head: PASS, mask 6. [T] N2: FAIL with merged route_mask = {2 true}, want DIRECT|FLOOD = 0110, matching the issue's probe (2). [T] The full cmd/ingestor suite under N2 fails only on this test; every _287 test still passes under N2.
3 Test-only, no production change — — [A] git diff origin/master --stat: one test file, 36 insertions. Production code is identical to master de6e83b4, so "green on master" is the head run.

Always-checks

Check Result Evidence
cmd/server stays read-only untouched [A]
No new map[string]interface{} outside tests 0 added [A]
No hardcoded colours no frontend change [A]
scripts/check-xss-sinks.sh --diff origin/master clean ("no public/**/*.{js,html} changes to scan") [T]
Fork guards 9 in deploy.yml, 1 in release-fast-path.yml (same as master) [T]

Local runs (head)

Suite Result Evidence
cmd/ingestor go test -count=1 -timeout 40m ./... ok [T]
sh test-all.sh 225 passed, 0 failed (225 files) [T]
node test-frontend-helpers.js 709 passed, 0 failed [T]
test-e2e-playwright.js against a local Go server on a fixture copy prepared as in CI (freshen, seed, migrate, seeds 2073/199/245); server stopped by pid and port afterwards 132/135 passed, 3 skipped by the script (2 flaky-marked, 1 Go perf test needing GO_BASE_URL) [T]

CI per job (head 4912821b)

Job Result Evidence
Go Build & Test pass [K]
Playwright E2E Tests pass [K]
Build & Publish Docker Image pass [K]
Release Artifacts skipped (PR) [K]
Deploy Staging skipped (PR) [K]
Publish Badges & Summary skipped (PR) [K]

No reruns; the known-unstable #271 did not fail.

Leftovers

  1. gofmt -l flags hash_migrate_route_mask_287_test.go on master too. gofmt would rewrite '' in an existing doc comment (the fill-column test, COALESCE(scope_name, '')) to a typographic quote. The added lines are gofmt-clean; the pre-existing line was left alone to keep the diff test-only and minimal. [T]
  2. cmd/server go test ./... was not run locally because nothing in it changed; CI "Go Build & Test" covers it. [A] [K]
  3. scripts/check-xss-sinks.sh has a bash shebang and fails under sh (Syntax error: "(" unexpected); run it with bash or directly. [T]

@adminopenclaw8-sketch

Copy link
Copy Markdown
Collaborator

Review — CS-Macmini PR#328 — head 4912821

Dom: APPROVE med nits

Independent, read-only review. Evidence: [T] = run locally by me on the merged tree (git merge-tree --write-tree origin/master <head> = fc4ef802, base bf3151a4), [A] = code reading, [K] = CI on this head.

Findings

# Sev Where Finding
N1 nit cmd/ingestor/hash_migrate_route_mask_287_test.go:287 The comment line "Master's COALESCE(route_mask, 0) merge ends at FLOOD here too" uses "Master" to mean the pre-#287 implementation. Current master carries the fix, and the test is green there — which is also what the acceptance criterion asks for. A reader who pairs this sentence with the issue's master = 2 row can conclude the test is red on master. The claim itself is correct for the pre-#287 code (survivor mask FLOOD, loser COALESCE(NULL,0) = 0, result non-NULL so the backfill skips the row → 2) [A]. The three sibling _287 comments carry the same wording, so this is file-wide, not introduced here.
N2 nit same file, new test The test pins the merged mask, the re-parented observation count and backfill stability, but — unlike ..._MergeRecomputesUnionWhenLoserUnknown_287 — not the route_mask_changes announcement row, and not that loser 131 is deleted. Neither is required by the acceptance criteria, and the change row is covered by the sibling, so this is a coverage observation only.
N3 note, out of scope test-e2e-playwright.js:285-295 Pre-existing and unrelated to this PR: the "Version info lives on Perf dashboard" skip-guard is !!(data.version || data.commit), which is truthy for the literal string "unknown", while renderVersionCard (public/perf.js:80-82) filters "unknown" out and returns ''. A server built without version ldflags and started outside a git checkout therefore hard-fails the test instead of taking the intended skip path. It cost me one E2E run; see Tests below. No action for this PR.

No blocking finding. No behaviour change: the diff is one test file.

Answers to the review points

1. The RZ1-shaped test is red under mutant N2 and green on master — run myself.
Verified [T].

  • Merged tree (production code byte-identical to master in cmd/ingestor/hash_migrate.go): PASS.
  • Mutant N2 (recomputeMergedRouteMask hoisted above the UPDATE OR IGNORE observations move, its result reused below): FAIL: merged route_mask = {2 true}, want DIRECT|FLOOD = 0110. Exactly the 2 the issue's probe predicts.
  • "Green on master" is a real run, not an inference: the merged tree carries current master's production code, and the test passes there [T].
  • I also ran the full cmd/ingestor suite under N2: it fails on exactly one test — the new one. Every other test, including all four _287 route-mask tests, still passes. That independently confirms the issue's premise (N2 survived the suite before) and the PR's claim [T].

2. The loser's observation header is the only evidence for the bit.
Verified both by reading and by experiment.

  • [A] Every other evidence term in the shape yields FLOOD only: survivor 130 route_type = 1 (FLOOD), loser 131 route_type = 1 (FLOOD), survivor's own observation header hm287FloodFrame, survivor's stored mask hm287FloodBit, loser's stored mask NULL. recomputeMergedRouteMask reads nothing else — two route_type reads plus substr(raw_hex,1,2) over the winner's observations — and the caller only ORs the two stored masks.
  • [T] Probe: on unmutated code, with the loser's observation header flipped from hm287DirectFrame to hm287FloodFrame and nothing else changed, the merged mask is exactly 2 (FLOOD). So no second DIRECT source exists in the fixture; removing that one header removes the bit.
  • [A] The comment's survival argument holds: idx_observations_dedup is UNIQUE(transmission_id, observer_idx, COALESCE(path_json,'')) (cmd/ingestor/db.go:459), and the loser's row differs from the survivor's in both observer_idx (o2 vs o1) and path_json (["bb"] vs ["aa"]), so the OR IGNORE move keeps it. The test's COUNT(*) = 2 assertion pins that empirically.

3. Test-only.
git diff origin/master...HEAD -- ':!*_test.go' is empty [T]. The whole diff is 36 added lines in cmd/ingestor/hash_migrate_route_mask_287_test.go; no other file, no deletions. cmd/server has no diff at all.

Always-checks

Check Result Evidence
Every acceptance criterion has a test, red before / green after Yes — one criterion, one test; "before" is mutant N2 (the PR has no production change to be red against) [T]
cmd/server read-only No diff under cmd/server [T]
New map[string]interface{} outside tests None added anywhere [T]
Hardcoded colours No frontend diff; the only #-match in the diff is the issue reference #324 [T]
scripts/check-xss-sinks.sh --diff origin/master clean: "no public/**/*.{js,html} changes to scan" (needs bash, not sh) [T]
Fork guards deploy.yml 9, release-fast-path.yml 1 — byte-identical to master, no workflow diff [T]
Closing keywords None in the commit message or the PR body ("Relates to #324") [T]
Commit author dborup <kontakt@meshview.dk>, committer likewise [T]
go vet ./... in cmd/ingestor clean [T]
gofmt -l Flags the file, but on master too, and the only hunk gofmt -d wants is the pre-existing COALESCE(scope_name, '') comment on line 184. 11 other cmd/ingestor files are flagged on the same toolchain, so this is a Go-version artefact, not the PR's. There is no gofmt gate in the workflows. The author's leftover #1 is accurate. [T]

Tests I ran (merged tree, base bf3151a4)

Suite Result
cmd/ingestor go test -count=1 -timeout 40m ./... ok (112 s) [T]
cmd/server go test -count=1 -timeout 40m ./... ok (53 s) [T]
New test -count=30 ok — not flaky [T]
go test -race -count=5 -run TestContentHashMigration_MergeRecomputes ok [T]
sh test-all.sh 225 passed, 0 failed (225 files) [T]
node test-frontend-helpers.js 709 passed, 0 failed [T]
node test-e2e-playwright.js against a local Go server on a fresh fixture copy prepared as in CI (freshen, grouped/Kpa-clawbot#1791 seed SQL, corescope-migrate, seeds 2073 / 199 / 245; instrumented public dir; server stopped by port) 132/135 passed, 3 skipped — same numbers the author reports [T]
node test-node-liveness-e2e.js 35 checks passed [T]

Two E2E detours, neither attributable to this PR and neither one of the known-unstable #256 / #267:

  • First run failed on "Version info lives on Perf dashboard". Root cause is N3 above plus my own harness: I review from a git archive export, so resolveCommit's git rev-parse fallback finds no repository and /api/health reports commit: "unknown". Replicating CI's .git-commit file fixed it, and the test then passed every time.
  • Second run failed on "Node side panel Details link navigates" ([data-loaded="true"] 15 s timeout) after the preceding nodes-page test had passed. It passed on the next run with no change — a local flake.

Mutants I ran myself

ID Mutation New test Verdict
N2 (the issue's) hoist recomputeMergedRouteMask above the UPDATE OR IGNORE observations move FAIL, mask 2 killed — and it is the only test in the whole suite that catches it
MA observation scan reads transmission_id = loser instead of winner FAIL, mask 2 killed
MB drop the UPDATE OR IGNORE re-parent entirely (loser's observations deleted) FAIL, mask 2 killed
MC routeMaskBitFromHeader's bit contributes nothing (mask |= 0 * …) FAIL, mask 2 killed
MD read only the winner's route_type (the old MX1) PASS expected survival — the shape makes both route_types FLOOD, and ..._MergeRecomputesBitFromLoserRouteType_287 kills it instead. Confirms the new test is narrowly targeted rather than overlapping its siblings.

CI per job (head 4912821b, run 37480948213, attempt 1, no reruns) [K]

Job Result
Go Build & Test success
Playwright E2E Tests success
Build & Publish Docker Image success
Release Artifacts skipped (PR)
Deploy Staging skipped (PR)
Publish Badges & Summary skipped (PR)

Head was 4912821b5db3a4b04dafa79cbd6ddb9fbeabd257 at both the start and the end of this review; the PR is still a draft and reports mergeable.

Not verified

@dborup
dborup marked this pull request as ready for review October 7, 2026 06:16
@dborup
dborup merged commit 9205f56 into master Oct 7, 2026
6 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.

3 participants