Skip to content

feat(nodes): separate flood and zero-hop adverts in node details - #2085

Draft
n30nex wants to merge 6 commits into
Kpa-clawbot:masterfrom
n30nex:codex/node-advert-kinds
Draft

n30nex wants to merge 6 commits into
Kpa-clawbot:masterfrom
n30nex:codex/node-advert-kinds

Conversation

@n30nex

@n30nex n30nex commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2073. Depends on backend support in #2088; keep this draft until that change lands.

Recent Adverts now groups the bounded sample in both node views as Flood, Mixed flood / zero-hop, Zero-hop, or Other / unknown. Each advert appears once, preserving reception metadata, ordering, packet links, sample counts and existing origin explanations.

The classifier consumes authoritative advert_kind from accumulated server evidence. It no longer guesses from the first stored frame or combines fields from different observations. Missing or unrecognized evidence stays unknown. The heading explains that older history may be incomplete.

Red correction: 766a87d (four assertion failures); green correction: 8edf36c.

Validation

  • Regression tests cover all kinds, missing/unknown fields, conflicting raw data, mixed grouping, both renderers and desktop/mobile layouts.
  • Combined real-ingestor proof: flood → zero-hop → flood overwrote the same observation. Both APIs and both node renderers retained one mixed advert; chart totals stayed unchanged.
  • Desktop/mobile screenshots inspected; no group overflow.
  • All 183 standalone suites passed (735 helper tests). ESLint: zero errors, 91 existing warnings; XSS and whitespace checks passed. Final CI passed at 8edf36c (run), including browsers and both container architectures.

Grouping is O(n) over at most 20 adverts. No additional requests, dependencies, configuration or theme values. OpenClaw browser profile was unavailable; local Chromium validation was used.

@n30nex
n30nex marked this pull request as ready for review September 27, 2026 22:19
@n30nex

n30nex commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Review feedback addressed (commit b6d2915):

  1. Each advert now has distinct observer/SNR/RSSI test data, checked beside its own Analyze link.
  2. Both empty groups must show their empty-state message.

Protocol and adversarial reviews found no production defects. Parent validation passed all 183 standalone suites and the complete core browser suite (133 passed, three existing fixture skips). Real API checks covered the desktop pane/full page and mobile full page. CI run 36353796690 recorded the expected five assertion failures before implementation; final-head CI is running.

@n30nex

n30nex commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

CI feedback addressed (commit 727abeb):

  1. Both node views now call the shared helper through window.groupRecentAdverts, matching its export and passing ESLint's no-undef gate.
  2. Re-ran the exact frontend lint command: zero errors (91 existing warnings), all 183 standalone suites, and real-API browser checks for the desktop pane/full page and mobile full page.

The earlier local checklist missed the complete ESLint gate; it is now included in validation. This changes only the two call sites and preserves the reviewed behavior.

@dborup

dborup commented Sep 28, 2026

Copy link
Copy Markdown
Contributor

I think there is one correctness gap in the classifier that is worth addressing before merge.

classifyRecentAdvert() classifies each deduplicated transmission from its single stored route_type / canonical raw_hex. In the current CoreScope ingestor, an existing transmission is looked up by packet hash, and later observations do not update transmissions.route_type or transmissions.raw_hex.

MeshCore's packet hash excludes the route header and path, so the same advert payload can be observed on different routes over time—for example, first as zero-hop and later as flood. In that case, the transmission retains the first-ingested routing evidence, so the UI will show the advert in only one group even though it was also seen on the other route.

This makes the result dependent on ingestion order and can under-report flood airtime. The fallback that combines the transmission's route_type with a selected observation's path_json has a similar provenance issue: those fields do not necessarily describe the same received frame.

The current tests use a different hash for every routing case, so they do not cover this scenario. I suggest adding a regression test where the same advert hash is received through both direct/zero-hop and flood routing, and classifying from accumulated per-transmission route evidence. Such a record should either be shown as mixed or otherwise clearly represented as having used both route families.

The wire-format parsing itself looks sound and matches the referenced MeshCore firmware; my concern is specifically the loss of later route evidence after transmission deduplication.

@n30nex

n30nex commented Sep 30, 2026

Copy link
Copy Markdown
Contributor Author

Review feedback addressed (commit 8edf36c):

  1. Node views now consume accumulated server advert_kind, including a single mixed group; canonical raw frames no longer override evidence.
  2. Missing evidence stays unknown, and both headings explain the history limitation.
  3. Regressions cover conflicting raw data, mixed/unknown groups, metadata and both layouts. All 183 standalone suites passed. Real ingestion of flood → zero-hop → flood retained one mixed advert in both APIs and desktop/mobile views.

This remains draft until backend #2088 lands. Its upgrade-time preservation correction is still under review; final-head CI is running.

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.

[feat] node detail, separate flood-adv from zero-hop adv

2 participants