Add machinetracker collapse-duplicates option by adobloug · Pull Request #4099 · Uninett/nav

adobloug · 2026-06-29T07:04:17Z

Scope and purpose

NAV records one Arp row per collecting router by design, so an address served by several routers (e.g. an HSRP pair) shows up as multiple overlapping rows in a machinetracker IP search for active addresses.

This adds an opt-in "Collapse duplicates" checkbox to the IP search form:

Default off → existing behavior unchanged (raw rows preserved).
When on → rows for the same (ip, mac) whose active time windows overlap are merged into one (newest kept).

Disjoint windows (genuine connect/disconnect history) are preserved — overlap test is s1 < e2 and s2 < e1. Still-active rows (end_time = INFINITY) always overlap, so they collapse to one.

This pull request

changes the UI (adds one checkbox to the machinetracker IP search form)

Contributor Checklist

How to observe

Have (or simulate) arp rows where the same (ip, mac) is collected by two routers — two rows, same ip/mac, both end_time = infinity, different sysname/netbox.
Go to Machinetracker → IP search, enter the prefix/address, filter = Active.
Without "Collapse duplicates": two rows per address. With it checked: one row.
Verify a genuine reconnect (same ip/mac, disjoint time windows) still shows both rows when collapse is on.

Trade-off

With collapse on, the kept row is the newest, so the dropped router's name in the Source column is not shown. Acceptable since the option is opt-in. A future enhancement could annotate "seen by N routers".

Screenshots

🤖 Generated with Claude Code

lunkwill42

Thanks for this — the design is sound and correctly opt-in, the disjoint-history preservation is right, and the collapsed row count is plumbed through to the counters correctly. I ran the machinetracker integration suite locally and it's green.

Having this as a single commit is borderline, but I'll allow it.

A few inline notes:

The collapse_overlapping / create_active_row suggestions are optional polish (a type annotation on the new helper, and the setdefault idiom) — the original codebase carries no type annotations, as those did not exist in Python at the time, but it's always good to see it on new code.
One I'd like before merge: the tests assert how many rows survive but never which one, so the headline "keep the newest" guarantee is currently untested (see the test-file comment).

Only other thing: the contributor checklist still has the before/after screenshots unchecked — since this adds a checkbox to the IP search form, could you attach those to the PR description?

lunkwill42 · 2026-07-09T12:32:59Z

    return result


+def collapse_overlapping(rows):


Optional: annotate the new helper. The rows are Arp instances (they come straight from get_arp_records), and your docstring already calls them "Arp result rows", so the type just formalises that:

Suggested change

def collapse_overlapping(rows):

def collapse_overlapping(rows: list[Arp]) -> list[Arp]:

If you take this, add Arp to the import on line 28 so the name resolves:
from nav.models.manage import Arp, Prefix, Netbox, Interface

lunkwill42 · 2026-07-09T12:32:59Z

    """Creates a tracker tuple where the result is active"""
    ip = str(ip_key)
    rows = ip_result[ip_key]
+    touched_keys = []


Optional: a set lets the dedup happen for free (pairs with the setdefault change below).

Suggested change

touched_keys = []

touched_keys = set()

lunkwill42 · 2026-07-09T12:32:59Z

+        key = (row.ip, row.mac)
+        if key not in tracker:
+            tracker[key] = []
+            touched_keys.append(key)
+        tracker[key].append(row)


Optional: setdefault is the idiomatic append-to-dict-of-lists, and with touched_keys as a set you no longer need the "first time only" guard:

Suggested change

key = (row.ip, row.mac)

if key not in tracker:

tracker[key] = []

touched_keys.append(key)

tracker[key].append(row)

key = (row.ip, row.mac)

tracker.setdefault(key, []).append(row)

touched_keys.add(key)

This is behaviour-preserving.

lunkwill42 · 2026-07-09T12:32:59Z

+            _row(now - timedelta(hours=1), INFINITY, "router-a"),
+            _row(now - timedelta(hours=2), INFINITY, "router-b"),
+        ]
+        assert len(collapse_overlapping(rows)) == 1


The whole point of the feature is that the newest row is the survivor, but nothing here asserts which row is kept — a collapse that kept the oldest would pass this test unchanged. Worth pinning down (router-a has the newer start_time):

Suggested change

assert len(collapse_overlapping(rows)) == 1

kept = collapse_overlapping(rows)

assert len(kept) == 1

assert kept[0].sysname == "router-a" # newer start_time wins

NAV records one Arp row per collecting router by design, so an address served by several routers (e.g. an HSRP pair) appears as multiple overlapping rows in a machinetracker IP search for active addresses. Add an opt-in "Collapse duplicates" checkbox to the IP search form. When enabled, rows for the same (ip, mac) whose active time windows overlap are collapsed to one (newest kept). Disjoint windows (genuine connect/disconnect history) are preserved. Default off, so existing behavior is unchanged. Note: the newest row is kept whole -- its own time window is shown, not a union of the duplicates' windows. If a dropped duplicate had a later end_time, that later end is not reflected. Fixes Uninett#4098 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Address review feedback on the collapse-duplicates option: - Pin the "keep newest" guarantee: the collapse test asserted only how many rows survived, not which one, so a collapse that kept the oldest would have passed. Assert the newer-start_time row wins. - Annotate collapse_overlapping() with list[Arp] -> list[Arp] and import Arp, formalising what the docstring already states. - Use the setdefault dict-of-lists idiom in create_active_row and track touched keys in a set, dropping the first-time-only guard. Behaviour-preserving. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sonarqubecloud · 2026-07-09T13:34:54Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

adobloug · 2026-07-09T13:43:53Z

Thanks for the review! Applied all of it:

Type annotation — collapse_overlapping(rows: list[Arp]) -> list[Arp] added, plus Arp in the import on line 28.
set + setdefault — touched_keys is now a set and create_active_row uses tracker.setdefault(key, []).append(row); dropped the "first time only" guard.
"keep the newest" now tested — the collapse test asserts kept[0].sysname == "router-a" (newer start_time), so a keep-oldest regression would fail.

While in there I also flagged a design caveat in the commit message: the newest row is kept whole — its own window is shown, not a union of the duplicates' windows, so a dropped duplicate's later end_time isn't reflected. In practice the newest row's window dominates for overlapping HSRP-style duplicates.

Rebased on latest master. Before/after screenshots attached to the PR description.

lunkwill42 assigned adobloug Jul 8, 2026

lunkwill42 self-requested a review July 9, 2026 11:36

lunkwill42 reviewed Jul 9, 2026

View reviewed changes

adobloug force-pushed the feature/machinetracker-collapse-duplicates branch from 98bbfa5 to ccf4c87 Compare July 9, 2026 13:15

adobloug and others added 2 commits July 9, 2026 15:33

adobloug force-pushed the feature/machinetracker-collapse-duplicates branch from ccf4c87 to d5bac8d Compare July 9, 2026 13:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add machinetracker collapse-duplicates option#4099

Add machinetracker collapse-duplicates option#4099
adobloug wants to merge 2 commits into
Uninett:masterfrom
adobloug:feature/machinetracker-collapse-duplicates

adobloug commented Jun 29, 2026 •

edited

Loading

Uh oh!

lunkwill42 left a comment

Uh oh!

lunkwill42 Jul 9, 2026

Uh oh!

lunkwill42 Jul 9, 2026

Uh oh!

lunkwill42 Jul 9, 2026

Uh oh!

lunkwill42 Jul 9, 2026

Uh oh!

sonarqubecloud Bot commented Jul 9, 2026

Uh oh!

adobloug commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	def collapse_overlapping(rows):
	def collapse_overlapping(rows: list[Arp]) -> list[Arp]:

-        assert len(collapse_overlapping(rows)) == 1
+        kept = collapse_overlapping(rows)
+        assert len(kept) == 1
+        assert kept[0].sysname == "router-a"  # newer start_time wins

Uh oh!

Conversation

adobloug commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Scope and purpose

This pull request

Contributor Checklist

How to observe

Trade-off

Screenshots

Uh oh!

lunkwill42 left a comment

Choose a reason for hiding this comment

Uh oh!

lunkwill42 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

lunkwill42 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

lunkwill42 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

lunkwill42 Jul 9, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jul 9, 2026

Quality Gate passed

Uh oh!

adobloug commented Jul 9, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adobloug commented Jun 29, 2026 •

edited

Loading