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

[saltas888]

Why

RelationshipManager.update()'s data parameter declared its collection member as an invariant
list[str | Node | dict | PeerWithRelationshipMetadata]. Because list is invariant in its element
type, a caller holding a narrower list[PeerWithRelationshipMetadata] could not pass it, which
forced a scoped suppression at the call site in core/manager.py:

# 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]

That suppression was not cosmetic: arg-type is not in infrahub.core.manager's
disable_error_code list, so the ignore was the only thing blinding mypy to that whole class of
error on a live code path. As the reviewer put it on #9977: "Since we are removing the warning, it's
always better to properly fix the warning."

Goal: type the parameter as a covariant, non-mutating Sequence so the suppression can go and
arg-type checking is restored there.

Non-goals: the unrelated # type: ignore[arg-type] on the **dict splat in core/manager.py,
and the one at menu/repository.py:105 (different root cause — a single CoreMenuItem | None, not a
list). Both deliberately untouched.

Tracking: INBOX-8 · originating review comment: #9977 (comment)

What changed

Behavioral changes — one, and it fixes a latent bug rather than altering existing behaviour:

• Passing a non-list sequence (e.g. a tuple) of peers is now correctly iterated as a
  collection. Previously such a value would have been wrapped as a single opaque "peer" and raised
  ValidationError. No current caller passes a tuple, so nothing observable changes today.

Implementation notes:

• data's collection member: list[...] → Sequence[...] (covariant, exposes no mutators).

• The runtime narrowing was widened in step with the annotation. This is the part worth
  reviewing: widening only the annotation would make tuple statically valid while
  isinstance(data, list) still routed it down the single-value branch — an annotation that accepts
  an input the runtime mishandles is a worse contract than the invariant list it replaced.

  # 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):

  str must be excluded first: str satisfies Sequence but is a legitimate single-value
  member of the union (a bare peer id). Without the carve-out, a peer id would be shredded into one
  peer per character — silently, since _process_update_item accepts str items. core/ipam/reconciler.py:168
  is a live caller that passes exactly that, so this is not hypothetical.

• dict needs no carve-out (a Mapping is not a Sequence), nor do Node,
  PeerWithRelationshipMetadata, or None.

• Sequence moved from typing to collections.abc (it appeared in only two places in the file),
  since typing.Sequence is deprecated and this change uses it as an isinstance target. The
  neighbouring Iterable/Iterator/Mapping imports were left alone on purpose — migrating
  them would be an unrelated drive-by refactor.

• The Sequence annotation is only sound because update() genuinely never mutates the collection —
  verified against the body, which iterates list_data once and never appends/assigns. (The nearby
  self._relationships.clear() acts on the manager's own collection, not the caller's argument.)

What stayed the same: no schema changes, no migrations, no GraphQL/REST contract changes, no auth
changes, no new dependencies, no CI changes, no generated files. Behaviour is unchanged for all seven
existing call sites.

How to review

Focus on one line — the widened narrowing in backend/infrahub/core/relationship/model.py. The
str-first ordering is what keeps peer ids from being iterated; everything else is mechanical.

The behaviour matrix this must preserve:

data	Before	After
list[...]	collection	collection
str	single	single (explicit carve-out)
dict / Node / PeerWithRelationshipMetadata / None	single	single (not a Sequence)
tuple[...]	single (wrong)	collection (fixed)

How to test

# The type error is genuinely gone
uv run mypy backend/infrahub/core/manager.py backend/infrahub/core/relationship/model.py

# New + existing relationship-manager coverage
uv run pytest backend/tests/component/core/test_relationship_manager.py

# Behaviour preservation on the bare-str path (diff/merge + the live ipam caller)
uv run pytest backend/tests/component/core/diff/merge
uv run pytest backend/tests/functional/ipam/test_ipam_rebase_reconcile.py \
              backend/tests/functional/ipam/test_ipam_merge_reconcile.py

Evidence from this branch:

Check	Result
mypy on both changed modules	clean
mypy --show-error-codes backend (full, as CI runs it)	clean — 1597 files
ruff check / ruff format --check	clean
invoke format, invoke main.lint, uv lock --check	clean
invoke backend.test-unit	2151 passed
test_relationship_manager.py	36 passed (34 existing, unmodified assertions + 2 new)
component/core/diff/merge	27 passed
functional/ipam rebase+merge reconcile	4 passed, 1 skipped
backend.validate-generated, schema.validate-graphqlschema, schema.validate-jsonschema, docs.validate	clean, no drift

Two checks confirm the change is meaningfully tested rather than vacuously passing:

1. Removing the ignore without the fix reproduces the original error — and mypy names the fix
   itself: "List" is invariant … Consider using "Sequence" instead, which is covariant.
2. Reverting only the narrowing (keeping the Sequence annotation) makes the new tuple test fail
   with ValidationError: Invalid data provided to form a relationship, proving the latent bug is
   real and the test catches it.

Not run locally: invoke backend.lint (the ty step) fails with 107 unresolved-import
diagnostics, but it fails identically on a pristine origin/develop tree in this environment and
none of the diagnostics touch the changed files — pre-existing and environment-caused, left to CI.
Frontend checks and docs.format were skipped (no frontend files changed; markdownlint-cli2 not
installed locally).

Impact & rollout

• Backward compatibility: fully compatible. The signature is relaxed, so every existing caller
  still type-checks and behaves identically. No migration steps.
• Performance: negligible — one extra O(1) isinstance per update() call.
• Config/env changes: none.
• Deployment notes: safe to deploy.

Checklist

• Tests added/updated
• Changelog entry added (changelog/+relationship-update-sequence-param.housekeeping.md)
• External docs updated (if user-facing or ops-facing change) — n/a, internal typing change
• Internal .md docs updated — spec/plan/research under dev/specs/005-manager-update-sequence-type/
• I have reviewed AI generated content

🤖 Generated with Claude Code

[cubic-dev-ai]

0 issues found across 1 file (changes from recent commits).

Confidence score: 5/5

• Automated review surfaced no issues in the provided summaries.
• No files require special attention.

<sub>Shadow auto-approve: would auto-approve. Type parameter widened from list to Sequence, runtime narrowing adjusted, and type suppression removed, with added tests preserving existing behavior.

Re-trigger cubic</sub>

[codspeed-hq]

Merging this PR will not alter performance

✅ 12 untouched benchmarks

---

_{Comparing pha/INBOX-8 (135a1de) with develop (75b5fe4)}

[saltas888]

CI triage: backend-tests-integration failure is a pre-existing flake, not from this PR

Every other check is green — including all unit (3.12/3.13/3.14), component (core-diff, core-schema, graphql, other), functional, docker-integration, E2E, benchmark, lint, and schema/docs validation jobs.

The single failure was:

FAILED backend/tests/integration/profiles/test_profile_lifecycle.py::TestProfileLifecycle::test_step_19_check_persons_again - Failed: DID NOT RAISE <class 'AttributeError'>
ERROR  backend/tests/integration/profiles/test_profile_lifecycle.py::TestProfileLifecycle::test_step_18_check_profile_for_removed_attribute - neo4j.exceptions.TransactionError: Transaction failed
1 failed, 481 passed, 113 warnings, 1 error

Root cause is step 18, not step 19. TestProfileLifecycle is a sequential, stateful class (step_1 … step_19). Step 18's schema migration died on a Neo4j TransactionError (Schema migration failed, beginning rollback in update_coordinator.py:418), so the attribute it was supposed to remove was still present — which is why step 19's pytest.raises(AttributeError) did not raise. Step 19 is collateral damage.

Evidence it is not caused by this PR:

1. The identical failure occurs on develop. Run 30384450091 (2026-07-28, commit "inject observer metrics for rate limiting (inject observer metrics for rate limiting #10052)") failed with the same two tests, the same TransactionError, the same DID NOT RAISE <class 'AttributeError'>, and the same counts — 1 failed, 481 passed, 1 error.
2. The job passes on later develop runs — 30433208789 and 30453014905 both ran backend-tests-integration to success, so the failure is intermittent rather than deterministic.
3. The failure mode is wrong for this change. This PR only relaxes a parameter annotation and widens an isinstance narrowing. A regression from it would surface as a TypeError / ValidationError / AttributeError in relationship handling — not as a Neo4j transaction failure inside a schema migration. All seven existing call sites pass a list, a bare str, a Node, or a PeerWithRelationshipMetadata; none passes a non-list sequence, so the one behavioural row that changed is unreachable from current code.

The branch is also 0 commits behind develop, so staleness is ruled out as a cause and no rebase was needed.

Action taken: re-ran only the failed job. No code changes — fixing the underlying flake would mean touching the schema-migration path, which is both outside this PR's scope and a governance-gated area for the automated pipeline. The flaky test looks worth its own ticket.

🤖 platform-health-agent reconcile pass

[ajtmccarty]

nice fix. out of scope for this PR, but PeerWithRelationshipMetadata is overused as both the write and read model for some relationship logic. so it is used to encode both responses from database queries AND updates for a Relationship. it should be split into 2 models: one for reading and one for writing

[ajtmccarty]

not sure if we need all this spec stuff for ~10 lines of production code change

[saltas888]

Fair point, and worth flagging up the chain rather than me deciding it on this PR.

These artifacts are the output of the speckit-opsmill-auto pipeline that the automated platform-health agent is required to run end-to-end, and .agents/rules/code-doc-style.md names dev/specs/ as their home (it's where the rule says ticket/spec IDs are allowed to live, as opposed to in code comments). So they're landing where the repo says they should — but you're right that seven process files for ~10 lines of production code is out of proportion.

That's a pipeline-policy call, not a this-PR call, so I've left them in place and raised it on INBOX-8 for a human to decide whether the agent should scale the speckit ceremony down (or skip the spec artifacts entirely) for Effort-S cards. Happy to strip them from this PR if you'd rather not carry them — just say so and I'll push the removal.

Leaving this thread unresolved so it stays visible for that decision.

[saltas888]

CI triage (run at 135a1dec): all failures are infrastructure, none attributable to this change

Re-ran the failed jobs. Breakdown of what was red and why:

Job	Reported	Actual cause
E2E-testing-pytest-playwright (foundation, branches_repo, sites_a)	fail	ERROR at setup — docker compose --project-name infrahub-test-… up --wait returned non-zero → RuntimeError: Failed to start docker compose. 201 deselected, 5 errors — the environment never came up, so no test ever ran.
backend-tests-component (graphql)	fail	OOM. [gw0] node down: Not properly terminated followed by Process completed with exit code 137 (SIGKILL). The xdist worker was killed; test_diff_tree_query.py::test_diff_tree_one_attr_change was merely the test in flight and got reported as collateral. No assertion output exists.
backend-tests-functional, backend-tests-component (core-diff), backend-benchmark	fail	Not failures — cancelled. The GitHub API reports "conclusion": "cancelled" for all three; they were killed by fail-fast when the jobs above went red. gh pr checks flattens cancelled into fail, which is what makes this run look far worse than it is.

Zero genuine test failures. Nothing here is reachable from a list → Sequence parameter annotation and an isinstance branch — a typing change cannot stop containers from starting or cause a runner OOM.

Corroborating evidence that the change itself is sound:

• The previous run on 98a1694 had all of these same suites green — unit (3.12/3.13/3.14), component (core-diff, core-schema, graphql, other), functional, docker-integration — with only a separate backend-tests-integration flake, already triaged above.
• CodSpeed on this PR: "Merging this PR will not alter performance", 12 untouched benchmarks.
• Locally on this branch: full-backend mypy clean (1597 files), backend.test-unit 2151 passed, test_relationship_manager.py 36 passed, component/core/diff/merge 27 passed, functional/ipam reconcile 4 passed.

On the branch being 4 commits behind develop

Noting it rather than acting on it. I deliberately did not rebase: the diagnosed causes are container startup and memory, neither of which staleness explains, and a force-push would risk dismissing @ajtmccarty's approval on a PR that is otherwise ready to go. mergeable is currently MERGEABLE, so there's no conflict. Happy to rebase if you'd rather have it fresh before merging — say the word.

🤖 platform-health-agent · automated reconcile pass