refactor(backend): accept any peer sequence in RelationshipManager.update

backend/infrahub/core/manager.py

@@ -1342,8 +1342,7 @@ async def _enrich_one_node_with_relationships(
                 rel_peers_with_metadata.append(peer_with_metadata)
 
             rel_manager.has_fetched_relationships = True
-            # invariant list parameter; update() only reads data, so the narrower element type is safe
-            await rel_manager.update(db=db, data=rel_peers_with_metadata)  # type: ignore[arg-type]
+            await rel_manager.update(db=db, data=rel_peers_with_metadata)
 
     @classmethod
     async def delete(

backend/infrahub/core/relationship/model.py

@@ -3,6 +3,7 @@
 import asyncio
 import copy
 import sys
+from collections.abc import Sequence
 from dataclasses import dataclass
 from typing import (
     TYPE_CHECKING,
@@ -11,7 +12,6 @@
     Iterator,
     Literal,
     Mapping,
-    Sequence,
     TypeVar,
     overload,
 )
@@ -1210,7 +1210,7 @@ async def get_relationship(self, db: InfrahubDatabase, peer_id: str) -> Relation
 
     async def update(
         self,
-        data: list[str | Node | dict[str, Any] | PeerWithRelationshipMetadata]
+        data: Sequence[str | Node | dict[str, Any] | PeerWithRelationshipMetadata]
         | dict[str, Any]
         | str
         | Node
@@ -1227,7 +1227,8 @@ async def update(
             ValidationError: When the supplied data cannot form a valid relationship.
 
         """
-        if not isinstance(data, list):
+        # A str satisfies Sequence but stands for a single peer id, so it must not be iterated.
+        if isinstance(data, str) or not isinstance(data, Sequence):
             list_data: Sequence[str | Node | dict[str, Any] | PeerWithRelationshipMetadata | None] = [data]
         else:
             list_data = data

backend/tests/component/core/test_relationship_manager.py

@@ -256,6 +256,44 @@ async def test_many_update(
     assert len(paths) == 2
 
 
+async def test_many_update_with_tuple(
+    db: InfrahubDatabase, tag_blue_main: Node, tag_red_main: Node, person_jack_main: Node, branch: Branch
+) -> None:
+    """A tuple of peers is treated as a collection of peers, like the equivalent list."""
+    person_schema = registry.schema.get(name="TestPerson")
+    rel_schema = person_schema.get_relationship("tags")
+
+    relm = await RelationshipManager.init(
+        db=db, schema=rel_schema, branch=branch, at=Timestamp(), node=person_jack_main
+    )
+    await relm.save(db=db)
+
+    await relm.update(db=db, data=(tag_blue_main, tag_red_main))
+    await relm.save(db=db)
+
+    peer_ids = {rel.peer_id for rel in await relm.get_relationships(db=db)}
+    assert peer_ids == {tag_blue_main.id, tag_red_main.id}
+
+
+async def test_many_update_with_peer_id_string(
+    db: InfrahubDatabase, tag_blue_main: Node, person_jack_main: Node, branch: Branch
+) -> None:
+    """A peer id is a single peer, never a sequence of one-character peer ids."""
+    person_schema = registry.schema.get(name="TestPerson")
+    rel_schema = person_schema.get_relationship("tags")
+
+    relm = await RelationshipManager.init(
+        db=db, schema=rel_schema, branch=branch, at=Timestamp(), node=person_jack_main
+    )
+    await relm.save(db=db)
+
+    await relm.update(db=db, data=tag_blue_main.id)
+    await relm.save(db=db)
+
+    peer_ids = {rel.peer_id for rel in await relm.get_relationships(db=db)}
+    assert peer_ids == {tag_blue_main.id}
+
+
 async def test_many_add(
     db: InfrahubDatabase, tag_blue_main: Node, tag_red_main: Node, person_jack_main: Node, branch: Branch
 ) -> None:

dev/specs/005-manager-update-sequence-type/alignment-check.md

@@ -0,0 +1,61 @@
+# Spec/Ask Alignment Check
+
+**Date**: 2026-07-29 | **Remediation passes used**: 0
+
+## Source
+
+The source-of-truth PRD is **Jira card [INBOX-8](https://opsmill.atlassian.net/browse/INBOX-8)**
+(Overview + Suggested solution + Scorecard), plus the linked originating discussion it was filed
+from: **[opsmill/infrahub#9977 review comment `discussion_r3614911929`](https://github.com/opsmill/infrahub/pull/9977#discussion_r3614911929)**
+by reviewer `polmichel`, fetched via `gh api` (not paraphrased from the card).
+
+Both were resolvable, so the check ran against the real sources rather than falling back to inline
+text.
+
+Card's stated ask, verbatim in substance:
+
+> **Overview**: `RelationshipManager.update()`'s data parameter is typed as an invariant list, which
+> forces a scoped `# type: ignore[arg-type]` at its caller in `core/manager.py`. Typing it as a
+> covariant `Sequence` lets the suppression be removed and keeps mypy meaningful on that path.
+>
+> **Suggested solution**: In `backend/infrahub/core/manager.py`, type the `update()` method's
+> collection parameter as `collections.abc.Sequence[...]` (non-mutating) and delete the `type:
+> ignore` comment that was suppressing the warning.
+
+Reviewer's guidance, verbatim:
+
+> Since we are removing the warning, it's always better to properly fix the warning. […] Sequence is
+> not mutable by default
+
+## Verdict
+
+✅ **ALIGNED**
+
+## Findings
+
+| Severity | Category | PRD reference | Spec reference | Description |
+|----------|----------|---------------|----------------|-------------|
+| — | (none: covered) | Suggested solution, clause 1 | FR-001 | Collection member re-typed as covariant `collections.abc.Sequence`. Present and faithful. |
+| — | (none: covered) | Suggested solution, clause 2 | FR-002 | `type: ignore` deleted. Present, and the spec correctly extends it to the adjacent explanatory comment, which becomes false once the parameter is covariant. |
+| — | (none: covered) | Reviewer: "Sequence is not mutable by default" | FR-004, research R2 | The spec makes non-mutation an explicit requirement and R2 *verified* it against the method body rather than assuming it. Faithful to the reviewer's actual rationale. |
+| ℹ️ Info | elaboration (not drift) | Not in PRD | FR-003, US3 | The spec adds a requirement that `update()`'s **runtime narrowing** be widened in step with the annotation. This is a **necessary clarification**, not added scope: the PRD's literal instruction, applied alone, makes `tuple` statically valid while the unchanged `isinstance(data, list)` test mishandles it (research R3). Implementing only what the card says would introduce a latent bug, so this is the card's own ask done correctly. |
+| ℹ️ Info | elaboration (not drift) | Not in PRD | T002, research R6 | Relocating the `Sequence` import from `typing` to `collections.abc`. Required to satisfy the PRD's own words (`collections.abc.Sequence`) and to make it a valid `isinstance` target. Two-line, contained. |
+| ℹ️ Info | non-goal added | Not in PRD | FR-006 | Spec explicitly excludes `menu/repository.py:105`'s separate `arg-type` ignore. This **narrows** rather than expands scope, and prevents a plausible misreading of "remove the type: ignore" as "remove all of them". |
+| ℹ️ Info | location precision | PRD says "In `backend/infrahub/core/manager.py`, type the `update()` method's collection parameter…" | plan D1/D3 | The card names `core/manager.py` as the edit site, but `update()` is **defined** in `backend/infrahub/core/relationship/model.py`; `core/manager.py` only *calls* it. The spec/plan split the change to the correct two files. This is a factual correction of an imprecise PRD, not drift — the card's intent (fix the signature, drop the ignore) is fully preserved. |
+
+## Assessment
+
+No requirement, goal, or non-goal from the PRD is **missing**, **softened**, **semantically
+changed**, or **contradicted**. Every spec addition is either a necessary consequence of executing
+the PRD correctly (FR-003, import relocation), a scope *reduction* (FR-006), or a factual correction
+of a file-location imprecision in the card. Acceptance criteria are strengthened, not dropped: the
+spec adds the revert-and-confirm verification (quickstart §2) because it discovered
+`warn_unused_ignores` is off and a clean mypy run alone would not prove the fix.
+
+Per the check's own rule, "cosmetic, structural, or expansion-of-detail differences are not drift;
+the spec is allowed to be longer, more precise, and to flesh out implicit requirements."
+
+## Action
+
+**Proceed** to implementation. No remediation pass needed; `spec.md` and `plan.md` are unchanged by
+this check.

dev/specs/005-manager-update-sequence-type/critique.md

@@ -0,0 +1,49 @@
+# Critique: `Sequence` typing for `RelationshipManager.update()`
+
+**Date**: 2026-07-29 | **Inputs**: `spec.md`, `plan.md`, `research.md` | **Lenses**: Product + Engineering
+
+## Findings
+
+| ID | Lens | Severity | Dimension | Finding | Recommendation |
+|----|------|----------|-----------|---------|----------------|
+| E1 | Engineering | 💡 | Type-narrowing correctness | mypy must narrow `data` to the `Sequence[...]` member in the `else` branch of `if isinstance(data, str) or not isinstance(data, Sequence):`. Because `str` is *both* a union member and itself a `Sequence`, this narrowing is not obviously going to land the way the design assumes — if mypy keeps `str` in the narrowed type, `list_data = data` may error. | Verify empirically during implementation (it is a 30-second mypy run). If narrowing misbehaves, the fallback is an explicit `else` assignment with a local variable typed to the collection member. Do **not** paper over it with a new `type: ignore` — that would defeat the card's entire purpose. |
+| E2 | Engineering | 💡 | Test coverage of the riskiest line | The `str` carve-out (research R4) is the highest-consequence line in the change — omitting it silently shreds a peer id into per-character peers. It *is* covered today, but only **incidentally**, by distant tests (`tests/component/core/diff/merge/_setup.py:160`, `diff_calculator/test_kind_migration.py:65`, `tests/integration/diff/*`) that pass a peer-id string as a side effect of testing diffs. Nothing near the relationship-manager tests asserts this contract on purpose. | Add an explicit single-`str` assertion alongside the new `tuple` test in `tests/component/core/test_relationship_manager.py`, so the carve-out is guarded locally and deliberately. Cheap (existing fixtures) and it documents the invariant where a future editor will actually see it. |
+| E3 | Engineering | 💡 | Scope of the import change | Relocating `Sequence` from `typing` to `collections.abc` (research R6) is strictly speaking beyond the card's literal text. It leaves `Iterable`, `Iterator`, and `Mapping` still imported from `typing` in the same block, which looks half-finished to a reviewer. | Keep the relocation (it is required to use `collections.abc.Sequence` as the card and reviewer specify) but do **not** migrate the neighbours — that is the drive-by refactor `.agents/rules/backend-component-design.md` explicitly warns against. Note the deliberate restraint in the PR body so the reviewer reads it as a choice, not an oversight. |
+| E4 | Engineering | 🤔 | Pre-existing annotation inconsistency | The local `list_data` annotation (`model.py:1231`) includes `| None` in its **element** type, while the `data` parameter's collection member does not. So `[None]` is representable internally but not accepted at the boundary — a pre-existing inconsistency this change inherits but does not cause. | Leave it. Resolving it means deciding whether a list containing `None` is legal input, which is a semantic question beyond an Effort-S typing fix. Not worth widening the card; flag only if a reviewer asks. |
+| P1 | Product | 💡 | Value framing | The card's user-visible value is nil — no behaviour changes for any current caller. Reviewed on its face, "removes a `type: ignore`" can read as churn. | Frame the PR around what the suppression was *hiding*: `arg-type` is live for `infrahub.core.manager` (research R8), so the ignore was blinding the type checker on a real code path. Also lead with the latent tuple bug the change fixes (research R3) — that is a concrete defect, not just hygiene. |
+| X1 | Both | 💡 | Scope discipline | The spec explicitly refuses two adjacent temptations: the second `arg-type` ignore in `menu/repository.py` (FR-006) and the `typing`→`collections.abc` migration of neighbouring imports (E3). | Correct calls, both. Keep them and state them in the PR body so the reviewer does not read the omissions as misses. |
+
+**No 🎯 Must-Address findings.**
+
+## Assessment by dimension
+
+- **Problem validation** — Strong. The request originates from a named reviewer on a merged PR
+  (#9977) who sketched the exact fix. No speculation about need.
+- **Scope × risk** — Well matched. Two production files, one test file. The plan identified the one
+  genuine risk (`str` ∈ `Sequence`) *before* implementation rather than discovering it in review.
+- **Constitution alignment** — Serves principle III (Type Safety) directly: it removes a
+  suppression rather than adding one, and tightens the parameter contract to non-mutating. Principle
+  IV is satisfied by the added test. No other principle engaged.
+- **Failure modes** — The two that matter are both identified and mitigated: char-wise `str`
+  iteration (R4, carve-out + E2 test) and tuple mishandling (R3, the narrowing fix itself).
+- **Verifiability** — Good, and honestly stated: the plan acknowledges `warn_unused_ignores` is off
+  and therefore prescribes a revert-and-confirm check (quickstart §2) rather than assuming a clean
+  mypy run proves anything.
+- **Soundness of the core move** — The `Sequence` substitution is only safe because the method does
+  not mutate the collection. Research R2 verified this against the body rather than assuming it,
+  which is the right order.
+
+## Actions taken
+
+E2 is folded into the task list as an explicit test task (single-`str` assertion next to the tuple
+test). E1 becomes a verification gate inside the implementation task rather than a separate task.
+E3, E4, P1, and X1 require no change to `spec.md` or `plan.md` — E3/X1 are notes for the PR body,
+E4 is a documented non-goal, P1 is framing.
+
+No 🎯 findings, so no spec/plan rewrite was needed.
+
+## Verdict
+
+✅ **PROCEED** — the spec and plan are sound, proportionate to an Effort-S card, and the one real
+technical hazard is identified with a mitigation before any code is written. Two 💡 items (E1, E2)
+are carried into the task list; the rest are PR-framing notes.