Skip to content

ADR-0033's prose contradicts itself and names a symbol that moved #454

Description

@bomly-guy

Done-criterion 1 of the SDK maturity program (dev-docs/SDK_MATURITY_PLAN.md §6) requires "ADR-0033 prose corrected". Phase 0.4 scheduled it as a doc-only change. It was never done, and I closed out the program without checking — filing so it does not disappear with the session that noticed.

The plan's own survey (line 94) recorded it:

ADR-0033's prose is stale: it still says EnsureNode "deliberately merges nothing"; #406 made scopes union. Supersede or annotate.

What is wrong, verified on main today

dev-docs/adr/0033-package-origin-is-detector-asserted.md, line 18, still reads:

Node insertion itself goes through one helper, detectors.EnsureNode, which deliberately merges nothing — records that differ deserve distinct nodes under distinct IDs, and records that are the same need nothing merged.

Three problems, and they have grown since the survey:

  1. It contradicts itself inside one paragraph. Twenty words earlier the same sentence-chain says "scope, relationship, and locations still union everywhere, being usage facts rather than assertions." Something that unions scope is merging.

  2. EnsureNode demonstrably merges now. In bomly-sdk v0.10.0 it is a thin wrapper over Graph.InsertNode, which folds by identity — unioning scopes, locations and origins, and merging relationship. "Merges nothing" was true of a helper that no longer exists in that form.

  3. The symbol moved. It is detectorkit.EnsureNode in the SDK; detectors.EnsureNode no longer exists in this repository. A reader following the ADR looks for something that is not there.

What to decide

The ADR's substance — origins are detector-asserted, contradicting records stay distinct, no tiebreak ever picks a winner — still holds and is worth keeping. Only the insertion-helper sentence is stale. So this is annotate-or-supersede, not rewrite:

  • Annotate with a dated note: identity folding unions usage facts (scope, relationship, locations, origins) and still never reconciles contradicting assertions, which is the distinction the ADR actually cares about. Cheapest, and keeps one document as the record.
  • Supersede with a new ADR if the fold semantics have moved far enough that the original reads as wrong rather than incomplete. ADR-0041 already owns identity; there may be nothing left for a new ADR to say.

Annotation looks right to me, but I have not read the ADR end to end — only the stale paragraph — so the person who does should make that call.

TestNodeInsertionGoesThroughTheSharedHelper in internal/detectors/guards_test.go still enforces the routing rule and its comment is accurate; it is only the ADR prose that drifted.

Activity

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

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions