Skip to content

Plan: per-observation route type — backfill and neighbor-builder use the transmission's first route type #289

Open

Description

@dborup

Found in a post-merge review of #190 and confirmed by reading master 34f672c6. This issue asks for a plan first, not code.

The same payload can be heard as both DIRECT and FLOOD. One transmission row then covers observations with different route types (route_mask records that). The resolved-path backfill (code) and the neighbor-builder (code) still read t.route_type, which is the route type of the first reception. That can store a wrong relay (DIRECT treated as FLOOD: path[last] is not the transmitter) or drop a real neighbour edge (FLOOD treated as DIRECT). The review reproduced both directions.

Plan should answer

  • Where does each observation's own route type come from today? Is it in raw_hex per observation, in path_json, or nowhere?
  • Does the fix need a schema change, such as an observations.route_type column with an ingest write and a backfill? What does it cost at the observation counts deployments have (millions of rows)?
  • Which consumers must switch: backfill, neighbor-builder, server-side resolve, analytics?
  • A migration and rollback plan, and tests for both reproduction directions.
  • Can a cheap interim guard limit the damage? For example, skip attribution when route_mask shows both DIRECT and FLOOD.

Activity

  1. added
    enhancementNew feature or request
    type:spikeResearch/investigation — produces a plan, not code
    on Oct 6, 2026
  2. dborup-agent commented on Oct 6, 2026

    @dborup-agent
    Collaborator

    Plan — CS-pve-agent3 #289

    Base: master 34f672c6. This is a plan only. No code is pushed and no PR is opened. The prototype I used to reproduce the bug and test the approach has been deleted.

    Evidence tags: [T] measured or tested here, [K] read in code or firmware source, [A] analysis or inference.

    Recommendation in one paragraph

    Take each observation's route type from its own frame header (observations.raw_hex, first byte, low two bits). This needs no schema change. When a row has no frame stored, fall back to t.route_type only if route_mask holds a single route family. If the mask holds both FLOOD and DIRECT, treat the route type as unknown: no observer anchor and no observer edge. That fallback rule is also the cheap interim guard. A separate, bounded repair pass fixes the resolved_path rows the backfill has already written with the wrong family.

    1. Where does an observation's own route type come from today?

    2. Is a schema change needed? Options and cost

    Option What Cost Verdict
    A. Derive from o.raw_hex Both queries also select substr(o.raw_hex,1,2) and t.route_mask; one helper picks the route type Builder batch of 50k rows: 56.9 → 67.8 ms. Backfill batch of 1k rows: 1.1 → 1.1 ms. Synthetic DB with 1M observations, warm cache, median of 7 runs [T]. No migration. Recommended
    B. New observations.route_type column, written at ingest, plus a backfill Schema change ALTER TABLE ADD COLUMN 18 ms. Backfill of 1M rows in 1,000-row batches: 13.0 s, writer hold p50 9.9 ms, p99 73 ms [T] (same synthetic DB). At the ~6.5M observations in the #89 sizing comment (route_mask_backfill.go:20) that is roughly 1.5 min of CPU plus cold-cache I/O, and every observation page is rewritten through the WAL [A]. Not recommended. It adds no information: it is derived from the same raw_hex and is NULL for the same pre-Kpa-clawbot#881 rows. It also needs rollback handling, because an older ingestor would insert NULLs, as with route_mask (route_mask_backfill.go:42-59).
    C. Guard only Skip the observer anchor and edge when route_mask has both families Two query columns, no migration Interim only. It drops correct attribution even when the header gives the answer. Rule 2 below contains it.

    Route-type rule (one helper, for example observationRouteType(header string, txRoute int, mask sql.NullInt64) int next to isFloodRoute in path_resolver.go):

    1. If the frame header parses, use its route bits.
    2. Otherwise, if route_mask is known and holds both FLOOD and DIRECT bits, return unknown (-1). Under-attribute rather than mis-attribute, the rule from PR fix(nodes): rebuild relay-hop history on startup from path_json Kpa-clawbot/CoreScope#1643.
    3. Otherwise use t.route_type. This keeps today's behaviour for pre-schema: store per-observation raw_hex (each observer sees different bytes on air) Kpa-clawbot/CoreScope#881 rows of single-family transmissions and for rows whose route_mask is still NULL during the Same advert seen on flood and zero-hop routes keeps the route of the first inserted observation #89 backfill.

    3. Which consumers must switch?

    Consumer Uses route type for Action
    resolved_path_backfill.go:259/307 (ingestor) observer anchor on the last hop Switch to the helper
    neighbor_builder.go:236/290 (ingestor) observer↔last-hop edge Switch to the helper
    Live ingest db.go:1225 observer anchor Already uses the frame's own type
    Server relay indexes (store.go:2119 indexObservationRelayHops) read persisted resolved_path No change. They get the fix through the column. [K]
    Server NeighborGraph read neighbor_edges No change. It gets the fix through the table. [K]
    Server fallback resolvers resolvePathForObs / resolvePathForObsColdLoad (neighbor_persist.go:121,171) no route type, no observer anchor No change [K]
    extractEdgesFromObs (neighbor_persist.go:253) adds an observer edge with no route gate at all No production caller (noted in #190). Leave it, or delete it in a cleanup. Out of scope.
    Per-transmission analytics: repeater_liveness.go:142 (unscoped relay count), classifyHopAnalyticsTransport (store.go:10541), memory breakdown (store.go:2875), route filter (store.go:3996) classify the transmission, not attribute a relay No switch in #289. Possible follow-up issue to use route_mask instead. Relay airtime already does (relay_airtime_share.go:226).

    cmd/server stays read-only and untouched. A server-side per-observation route type would need another source anyway, because live-polled StoreObs deliberately does not keep RawHex (store.go:3688). [K]

    4. Milestones, migration and rollback

    M1: switch both consumers (one PR, code only, no schema).

    • Add the helper. Both queries select COALESCE(substr(o.raw_hex,1,2),'') and t.route_mask. The new values are passed into resolveObservationPath and isFloodRoute.
    • After deploy, new backfill batches and new builder batches are correct.
    • Rollback: revert the PR. There is no schema or data to undo.

    M2: repair already-written resolved_path (separate PR, after sizing).

    • Candidates: live ingest used the correct type, so only rows whose header family differs from t.route_type's family can be wrong. Both directions are covered:
      • DIRECT anchored as FLOOD stores a wrong relay;
      • FLOOD treated as DIRECT leaves resolved_path NULL, and the backfill watermark has already moved past the row, so a corrected backfill never revisits it. [T] The repair must therefore include NULL rows below the watermark, not only non-NULL ones.
    • Size it first with a read-only count on a copy:
      SELECT COUNT(*) FROM observations o JOIN transmissions t ON t.id = o.transmission_id
      WHERE length(o.raw_hex) >= 2
        AND ((instr('0123456789abcdef', lower(substr(o.raw_hex,2,1))) - 1) & 2) != (t.route_type & 2)
        AND COALESCE(o.path_json,'') NOT IN ('', '[]');
      (& 2 separates the families: 0 and 1 are FLOOD, 2 and 3 are DIRECT.)
    • Mechanism: an ingestor async migration (RunAsyncMigration, writerMu, bounded batches, the Same advert seen on flood and zero-hop routes keeps the route of the first inserted observation #89 pattern) that re-resolves the candidates with the correct type and overwrites resolved_path. The resolve runs outside the writer transaction (the flaky: TestResolvedPathBackfill_WriteHoldUnderBudget_188 — wall-clock write-hold budget fails under CI load #267 pattern).
    • Server visibility: the server sees repaired rows at its next Load, because the live poll only picks up new ids. I recommend restart-only, documented. A change log like route_mask_changes is only worth adding if the count is large.
    • Rollback: the migration has an _async_migrations marker. Reverting the code stops further repair. Rows already repaired stay repaired, and they are more correct than before. No down-migration is needed.
    • neighbor_edges is not repaired. False DIRECT-derived observer edges age out with the 5-day prune unless they are refreshed. Dropped edges come back with new flood traffic. This is the same approach feat(ingestor): resolve the last hop from the observer (#188) #190 took. [K/A]

    5. Cheap interim guard

    Rule 2 is the guard: skip when the mask is mixed and the row has no header. On its own it is about ten lines over two queries, and t.route_mask already exists. M1 costs only one more substr column, so I recommend shipping M1 directly with the guard as its fallback branch. If a two-step rollout is preferred, the guard alone can go first.

    Measurements on e2e-fixture.db (read-only)

    Fixture prepared as in CI: freshen, corescope-migrate, the three seed SQL files. [T]

    Metric Value
    transmissions / observations 531 / 532
    observations per transmission 530 × 1, 1 × 2
    t.route_type (0 / 1 / 2) 1 / 386 / 144
    transmissions with both DIRECT and FLOOD in route_mask 2, both synthetic seed-2073 adverts with an empty path, so 0 rows with a non-empty path
    observations with raw_hex 0 (all pre-Kpa-clawbot#881)
    observations whose header family differs from t.route_type (M2 candidates) 0

    The fixture cannot show the real-world rate. It has one observation per transmission and no per-observation frames. Sizing needs the count query above, run on a production-like copy by someone with access.

    Reproduction on master (prototype, deleted)

    These are throwaway tests on cmd/ingestor that I ran against 34f672c6 and then against a prototype of rules 1–3. [T]

    Test Scenario master prototype
    Backfill: DIRECT observation under a FLOOD transmission header 0x16, path ["c3"], t.route_type=1, mask 0b0110 red: stores c355…aa via the observer anchor green
    Backfill: FLOOD observation under a DIRECT transmission header 0x15, path ["c3"], t.route_type=2, mask 0b0110 red: resolved_path stays NULL (separate run, no prototype fix applied) not run
    Builder: FLOOD observation under a DIRECT transmission header 0x15, path ["c366"], t.route_type=2 red: real observer↔c366 edge dropped green
    Builder: DIRECT observation under a FLOOD transmission header 0x16, path ["c366"], t.route_type=1 red: false observer↔c366 edge green
    Guard: mixed mask, no header raw_hex NULL, mask 0b0110 not run (expected red: master anchors on t.route_type=1) [A] green
    Fallback: single-family or NULL mask, no header raw_hex NULL, mask 0b0010 or NULL not run (expected green: unchanged path) [A] green

    Mutants on the prototype:

    Mutant Caught by
    M1 builder reads t.route_type both builder tests + guard
    M2 backfill reads t.route_type backfill test + guard
    M3 guard removed guard
    M4 missing header always unknown fallback
    M5 / M6 wrong header slice (substr(…,2,2)) in backfill / builder backfill / builder DIRECT test
    M7 guard checks only the DIRECT bits survives. It is equivalent for both consumers, because a DIRECT-only mask already means "not flood". The helper's unit table (T7 below) kills it.

    With the prototype, the full cmd/ingestor suite passed (ok, 1258 s). It needs -timeout 25m on this host; the default 10 min is too short. [T]

    Tests for the implementation

    Each test must be red before its change and green after, with at least one mutant per point. All tests are in cmd/ingestor.

    # Test Red before because Mutant it must kill
    T1 Backfill does not anchor a DIRECT observation under a FLOOD transmission stores the observer-adjacent relay backfill reads t.route_type; wrong header slice
    T2 Backfill anchors a FLOOD observation under a DIRECT transmission resolved_path stays NULL [T] same
    T3 Builder adds an observer edge for a FLOOD observation under a DIRECT transmission edge dropped builder reads t.route_type
    T4 Builder adds no observer edge for a DIRECT observation under a FLOOD transmission false edge same; wrong header slice
    T5 Mixed mask with no header: no anchor, no observer edge (both consumers) anchors / adds edge guard removed
    T6 Single-family or NULL mask with no header keeps today's result pins no regression (green before and after) missing header always unknown
    T7 Helper unit table: all four header route types, upper- and lower-case hex, short or invalid header, masks 0b0011 / 0b1100 / mixed / NULL helper does not exist yet guard checks one family only (M7)
    T8 (M2) Repair re-resolves a family-mismatch row (both a wrong relay and a NULL row below the watermark), leaves matching rows untouched, writes 0 on a second run, and does not resolve while holding the writer transaction no repair exists drop the family condition; resolve under the lock (#267-style hook)

    There is no UI change. The existing E2E suites against the CI-prepared fixture must stay green.

    Constraints

    • cmd/server is not touched and stays read-only.
    • No new map[string]interface{}, no frontend, no colours, so the XSS gate has nothing to scan.
    • No new config values, so nothing for the customizer.
    • Fork guards are unchanged: 9 in deploy.yml, 1 in release-fast-path.yml.

    Performance: M1 adds one 2-character column read per row in batches that are already bounded (neighborBuilderMaxBatch = 50000, backfill batch size). That is O(1) per row and about +19% on a 50k-row builder batch (+11 ms) [T]. It runs off the ingest hot path.

    Residuals (outside #289)

    • Shared upsert row: one observer hearing DIRECT and FLOOD with an identical path shares one observation row, and the last frame wins. Fixing that means changing the upsert key, which rebuilds a unique index over millions of rows. Track it separately if the count above shows it matters.
    • Pre-schema: store per-observation raw_hex (each observer sees different bytes on air) Kpa-clawbot/CoreScope#881 rows with a single-family mask: the mask is a lower bound (route_mask_backfill.go:15-19). A variant whose frame no longer exists cannot be detected. These rows keep today's behaviour and age out with retention.
    • Per-transmission analytics: the classifiers listed in section 3 could read route_mask. That is a possible follow-up issue.
  3. added a commit that references this issue on Oct 6, 2026
  4. dborup commented on Oct 6, 2026

    @dborup
    OwnerAuthor

    M1 merged via #308 (298609e06e4e4c3ed6bdc37661abd176ee97bc68): the resolved-path backfill and the neighbour builder now use each observation's own route type. M2 (repairing already-written resolved_path rows) is still open, so this issue stays open.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or requesttype:spikeResearch/investigation — produces a plan, not code

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions