Name the NetBox row a recorded merge rejection is about - #207
Merged
captainpacket merged 1 commit intoAug 15, 2026
Merged
Conversation
This was referenced Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the second half of #206.
What an operator sees
The field report in #206 quotes this record verbatim:
Accurate, and unactionable — resolving it means editing the offending row in NetBox, and
one
ipam.ipaddressrow out of ~4,000 devices is being talked about with nothing sayingwhich. After this patch the same record ends:
raw_datagainsrow_pkandrowalongside the existing diagnosis keys.The identity was never missing
_record_failedalready holds theCollapsedChange, whosekeyis(label_lower, pk)— the same
key[1]the authoritative-delete guard logs in the clear a few frames below.It just was not passed to
record(), which tookmodel_stringandexcand nothingelse.
record()gainspkandchange_data, both optional.It names the pk, not the address — deliberately
#206 asks for the address, "and ideally the device and interface". I did not do that, and
the reason is your own invariant rather than an oversight:
diagnostic_shapekeeps field names and discards their values;redacted_message_shapeexists for the same reason;_MergeIssueRecorder's own comment says the key values a PostgresDETAILline embeds"are deliberately still not captured".
An address, a device name and an interface name are customer values, and ingestion issues
reach support bundles. The NetBox pk is a surrogate you assigned; it resolves to the
object page, which is exactly where the edit has to happen, and it discloses nothing. So
it answers "which row" completely and leaves the boundary intact.
If you want the value as well, that is one function.
_row_identityis the only placeto change, and
MergeRowIdentityTests.test_no_customer_value_reaches_the_message_or_the_shapeplus the two
assertNotInpairs in the recorder tests are the assertions that would needto be retired — knowingly, which is the point of putting them there.
The Drift Summary half of #206 is not addressed here
I confirmed the diagnosis and stopped.
EXACT_COMPARISONis defined and never produced:_dependency_dry_run_payloadhardcodesworkload_upper_bound(
views.py:507-508,:563) anddrift_report.py:136,176gates on the constant, withall(...)across models, so one upper-bound model disables the whole report.Your fix direction offers two answers — compute a real per-model comparison, or stop
presenting a drift measurement on a path that cannot produce one and relabel the panel.
That is a product decision. Picking one uninvited on a first contribution seemed worse
than leaving it, so please keep #206 open for it; I am happy to take whichever
direction you choose.
Compatibility
pkdefaults toNoneand_row_identityreturnsNonefor it, so a caller with no rowin hand — the orchestration paths that record a whole-model failure — gets the message it
always had, byte for byte.
MergeIssueRecorderTest.test_module_bay_failures_are_recorded_as_blocking_merge_issuesstill asserts
"Merge for dcim.modulebay failed (Exception)."by equality, unchanged, andis the regression test for that path. New
raw_datakeys are disjoint from the diagnosiskeys. No migration —
raw_datais already aJSONField.The trailing sentence is appended rather than folded into the prefix, because
safe_operation_failurewrites the head thatrecovered_classifiersreads back out.docs/03_Plans/active/2026-08-15-name-the-merge-rejected-row.mdis in the diff becausescripts/check_harness.pyrequires a plan for a high-risk surface; it failed on the changewithout one.
What I verified, and what I could not
Ran and green:
black 25.9.0 --check,flake8 7.3.0,scripts/check_harness.py,scripts/check_sensitive_content.py._row_identityis exercised against your realdiagnostic_shape— no pk leaves the message untouched, an int pk renders, a UUID pksurvives, a DELETE with no
postchange_datarecords no shape, and no customer valuereaches either surface.
I could not run
invoke test. It needs NetBox, PostgreSQL and Redis, and I do not havethat runtime — so the four tests added here are written against the suite's existing
fixtures and conventions but have not been executed. Please treat CI, not this PR body, as
the authority on them.
Tests added:
MergeRowIdentityTests(4,SimpleTestCase),test_the_skipped_row_names_the_netbox_object_to_edit(end-to-end throughsync_mergewith the field-reported
primary-ip-reassignment-blocked), andtest_a_recorded_row_names_its_pk_and_records_its_shape(pins the exact message andraw_data).